Refactor the code that calculates the offsets for the various pieces of
the DWARF .debug_names index section, to make it easier to share the
code with other tools, such as LLD.
Spent a bunch of time tracing down an odd issue "in SCEV" which turned out
to be the fact that SCEV doesn't have access to TTI. As a result, the only
way for it to get range facts on vscales (to avoid collapsing ranges of
element counts and type sizes to trivial ranges on multiplies) is to look
at the vscale_range attribute. Since vscale_range is set by clang by
default, manually setting it in the tests shouldn't interfere with the
test intent.
Patch 2 of 3 to add llvm.dbg.label support to the RemoveDIs project. The
patch stack adds the DPLabel class, which is the RemoveDIs
llvm.dbg.label
equivalent.
1. Add DbgRecord base class for DPValue and the not-yet-added
DPLabel class.
-> 2. Add the DPLabel class.
3. Enable dbg.label conversion and add support to passes.
This will be used (and tested) in the final patch(es), coming next.
This fixes 73c646a3b27293f8cb4ba120de7bc01c223b4b5f.
I misread the #ifdefs and didn't realise that they were in
the middle of passing parameters to a function.
Move the workaround outside this.
Similar to #82200:
Add paths for SystemZ related changes to the PR labeler.
There is no pr-subscribers-backend:SystemZ team in the llvm org yet.
Much appreciated if some admin can help to create the team.
This patch fixes#72748 by modifying the processing of program units to
search for a symbol to which OpenMP REQUIRES clauses can bind to. Rather
than picking up the first PFT node with a source reference and getting
its associated scope, it picks up the last one.
This avoids using the source from the first specification construct of
a nameless program, which can sometimes not be associated to any scope,
causing an ICE due to an invalid source location.
Currently we emit errors whenever a map is not provided on a target data
directive, however, I believe that's incorrect behavior, the
specification states:
"At least one map, use_device_addr or use_device_ptr clause must appear
on the directive"
So provided one is present, the directive is legal in this case.
Slightly different to its siblings (enter/exit/update) which don't have
use_device_addr/use_device_ptr.
When checking whether we need to build libc-hdrgen, we need to check
LLVM_ENABLE_RUNTIMES and RUNTIMES_${target}_LLVM_ENABLE_RUNTIMES, just
the former is not sufficient since libc may be enabled only for certain
targets.
This fixes the case where we would shrink an frem to half and then
bitcast to bfloat, producing invalid results. The transformation was
written under the assumption that there is only one type with a given
bit width.
Also add a strategic assert to CastInst::CreateFPCast to turn this
miscompilation into a crash.
The deduplication of the calls to `omp_get_thread_limit` used to be
legal when originally added in
<e28936f613 (diff-de101c82aff66b2bda2d1f53fde3dde7b0d370f14f1ff37b7919ce38531230dfR123)>,
as the result (thread_limit) was immutable.
However, now that we have `thread_limit` clause, we no longer have
immutability; therefore `omp_get_thread_limit()` is not a deduplicable
runtime call.
Thus, removing `omp_get_thread_limit` from the
`DeduplicableRuntimeCallIDs` array.
Here's a simple example:
```
#include <omp.h>
#include <stdio.h>
int main()
{
#pragma omp target thread_limit(4)
{
printf("\n1:target thread_limit: %d\n", omp_get_thread_limit());
}
#pragma omp target thread_limit(3)
{
printf("\n2:target thread_limit: %d\n", omp_get_thread_limit());
}
return 0;
}
```
GCC-compiled binary execution: https://gcc.godbolt.org/z/Pjv3TWoTq
```
1:target thread_limit: 4
2:target thread_limit: 3
```
Clang/LLVM-compiled binary execution:
https://clang.godbolt.org/z/zdPbrdMPn
```
1:target thread_limit: 4
2:target thread_limit: 4
```
By my reading of the OpenMP spec GCC does the right thing here; cf.
<https://www.openmp.org/spec-html/5.2/openmpse12.html#x34-330002.4>:
> If a target construct with a thread_limit clause is encountered, the
thread-limit-var ICV from the data environment of the generated initial
task is instead set to an implementation defined value between one and
the value specified in the clause.
The common subexpression elimination (CSE) of the second call to
`omp_get_thread_limit` by LLVM does not seem to be correct, as it's not
an available expression at any program point(s) (in the scope of the
clause in question) after the second target construct with a
`thread_limit` clause is encountered.
Compiling with `-Rpass=openmp-opt -Rpass-analysis=openmp-opt
-Rpass-missed=openmp-opt` we have:
https://clang.godbolt.org/z/G7dfhP7jh
```
<source>:8:42: remark: OpenMP runtime call omp_get_thread_limit deduplicated. [OMP170] [-Rpass=openmp-opt]
8 | printf("\n1:target thread_limit: %d\n",omp_get_thread_limit());
| ^
```
OMP170 has the following explanation:
https://openmp.llvm.org/remarks/OMP170.html
> This optimization remark indicates that a call to an OpenMP runtime
call was replaced with the result of an existing one. This occurs when
the compiler knows that the result of a runtime call is immutable.
Removing duplicate calls is done by replacing all calls to that function
with the result of the first call. This cannot be done automatically by
the compiler because the implementations of the OpenMP runtime calls
live in a separate library the compiler cannot see.
This optimization will trigger for known OpenMP runtime calls whose
return value will not change.
At the same time I do not believe we have an analysis checking whether
this precondition holds here: "This occurs when the compiler knows that
the result of a runtime call is immutable."
AFAICT, such analysis doesn't appear to exist in the original patch
introducing deduplication, either:
-
9548b74a83
- https://reviews.llvm.org/D69930
The fix is to remove it from `DeduplicableRuntimeCallIDs`, effectively
reverting the addition in this commit (noting that `omp_get_max_threads`
is not present in `DeduplicableRuntimeCallIDs`, so it's possible this
addition was incorrect in the first place):
- [OpenMP][Opt] Annotate known runtime functions and deduplicate more,
-
e28936f613 (diff-de101c82aff66b2bda2d1f53fde3dde7b0d370f14f1ff37b7919ce38531230dfR123)
As a result, we're no longer unsoundly deduplicating the OpenMP runtime
call `omp_get_thread_limit` as illustrated by the test case: Note the
(correctly) repeated `call i32 @omp_get_thread_limit()`.
---------
Co-authored-by: Joseph Huber <huberjn@outlook.com>
NOTE: For brevity the following talks about ConstantInt but
everything extends to cover ConstantFP as well.
Whilst ConstantInt::get() supports the creation of vectors whereby
each lane has the same value, it achieves this via other constants:
* ConstantVector for fixed-length vectors
* ConstantExprs for scalable vectors
However, ConstantExprs are being deprecated and ConstantVector is
not space efficient for larger vector types. By extending ConstantInt
we can represent vector splats by only storing the underlying scalar
value.
More specifically:
* ConstantInt gains an ElementCount variant of get().
* LLVMContext is extended to map <EC,APInt>->ConstantInt.
* BitcodeReader/Writer support is extended to allow vector types.
Whilst this patch adds the base support, more work is required
before it's production ready. For example, there's likely to be
many places where isa<ConstantInt> assumes a scalar type. Accordingly
the default behaviour of ConstantInt::get() remains unchanged but a
set of flags are added to allow wider testing and thus help with the
migration:
--use-constant-int-for-fixed-length-splat
--use-constant-fp-for-fixed-length-splat
--use-constant-int-for-scalable-splat
--use-constant-fp-for-scalable-splat
NOTE: No change is required to the bitcode format because types and
values are handled separately.
NOTE: For similar reasons as above, code generation doesn't work
out-the-box.
To fix https://github.com/llvm/llvm-project/issues/81597, I'm planning
to refactor the usage of CallDescription; and as I was preparing for
this I noticed that there are two superfluous references to this header.
A memory access is an out of bounds error if the offset is < the extent
of the memory region. Notice that here "<" is a _mathematical_
comparison between two numbers and NOT a C/C++ operator that compares
two typed C++ values: for example -1 < 1000 is true in mathematics, but
if the `-1` is an `int` and the `1000` is a `size_t` value, then
evaluating the C/C++ operator `<` will return false because the `-1`
will be converted to `SIZE_MAX` by the automatic type conversions.
This means that it's incorrect to perform a bounds check with
`evalBinOpNN(State, BO_LT, ...)` which performs automatic conversions
and can produce wildly incorrect results.
ArrayBoundsCheckerV2 already had a special case where it avoided calling
`evalBinOpNN` in a situation where it would have performed an automatic
conversion; this commit replaces that code with a more general one that
covers more situations. (It's still not perfect, but it's better than
the previous version and I think it will cover practically all
real-world code.)
Note that this is not a limitation/bug of the simplification algorithm
defined in `getSimplifedOffsets()`: the simplification is not applied in
the test case `test_comparison_with_extent_symbol` (because the `Extent`
is not a concrete int), but without the new code it would still run into
a `-1 < UNSIGNED` comparison that evaluates to false because
`evalBinOpNN` performs an automatic type conversion.
\llvm\flang\lib\Evaluate\fold-integer.cpp(705,35): warning: lambda capture 'FromInt64' is not used [-Wunused-lambda-capture]
It is intentionally unused.
It's important for PHI nodes because if a PHI node has multiple edges
coming from the same block, we can have the same incoming value multiple
times in the list of incoming values. All of those need to be consistent
(exact same Value*) otherwise verifier complains.
Fixes SWDEV-445797
This patch removes the `-reverse-csr-restore-seq` option from
AArch64FrameLowering, since this is no longer used.
This patch was reverted because of a crash in PR#79623.
Merging it back as it was fixed in PR#82492.
Now that there is no special checking for valid DPP encodings, these
instructions can use the same DecoderNamespace as other 64- or 96-bit
instructions.
Also clean up setting DecoderNamespace: in most cases it should be set
as a pair with AssemblerPredicate.
The traditional promotion is known to generate wrong code.
Like #80440 for ARM, except that far less is affected as on AArch64,
hardware floating point support always includes FP16 support and is
unaffected by these changes. This only affects `-mgeneral-regs-only`
(Clang) / `-mattr=-fp-armv8` (LLVM).
Because this only affects a configuration where no FP support is
available at all, `useFPRegsForHalfType()` has no effect and is not
specified: `f32` was getting legalized as a parameter and return type to
an integer anyway.
This fails on one of our bots:
https://lab.llvm.org/buildbot/#/builders/120/builds/6309
llvm-jitlink error: Unsupported target machine architecture in COFF object
The other bot doesn't run the test at all it seems but I can't explain
why. It's also possible that I'm mistaken and the mostly native but still
"cross compiling" setup we have on WoA means an x86 object is produced sometimes
(perhaps because a default triple is still x86).
Accumulator operand of MADD instruction can be bypassed from another
MUL-like operation. Before this fix bypassing was incorrectly applied to
multiplier operand.
Co-authored-by: Yury Gribov <gribov.yuri@huawei.com>
Split Dpp8FI and Dpp16FI into two different operands sharing an
AsmOperandClass. They are parsed and rendered identically as fi:1 but
the encoding is different: for DPP16 FI is a single bit, but for DPP8 it
uses two different special values in the src0 field. Having a dedicated
decoder for Dpp8FI allows it to reject other (non-special) src0 values
so that AMDGPUDisassembler::getInstruction no longer needs to call
isValidDPP8 to do post hoc validation of decoded DPP8 instructions.
This PR is to prevent creation of jump tables from switch. The reason is
that SPIR-V doesn't know how to lower jump tables, and a sequence of
commands that IRTranslator generates for switch via jump tables breaks
SPIR-V Backend code generation with complains to G_BRJT. The next
example is the shortest code to break SPIR-V Backend code generation in
this way:
```
target datalayout = "e-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-n8:16:32:64"
target triple = "spir64-unknown-unknown"
define spir_func void @foo(i32 noundef %val) {
entry:
switch i32 %val, label %sw.epilog [
i32 0, label %sw.bb
i32 1, label %sw.bb2
i32 2, label %sw.bb3
i32 3, label %sw.bb4
]
sw.bb:
br label %sw.epilog
sw.bb2:
br label %sw.epilog
sw.bb3:
br label %sw.epilog
sw.bb4:
br label %sw.epilog
sw.epilog:
ret void
}
```
To resolve the issue we set a high lower limit for number of blocks in a
jump table via getMinimumJumpTableEntries() and prevent undesirable (or
rather unsupported at the moment) path of code generation.
This commit is a refactoring of the dialect conversion. The dialect
conversion maintains a list of "IR rewrites" that can be committed (upon
success) or rolled back (upon failure).
Until now, the signature conversion of a block was only a "partial" IR
rewrite. Rollbacks were triggered via
`BlockTypeConversionRewrite::rollback`, but there was no
`BlockTypeConversionRewrite::commit` equivalent.
Overview of changes:
* Remove `ArgConverter`, an internal helper class that kept track of all
block type conversions. There is now a separate
`BlockTypeConversionRewrite` for each block type conversion.
* No more special handling for block type conversions. They are now
normal "IR rewrites", just like "block creation" or "block movement". In
particular, trigger "commits" of block type conversion via
`BlockTypeConversionRewrite::commit`.
* Remove `ArgConverter::notifyOpRemoved`. This function was used to
inform the `ArgConverter` that an operation was erased, to prevent a
double-free of operations in certain situations. It would be unpractical
to add a `notifyOpRemoved` API to `IRRewrite`. Instead, erasing
ops/block should go through a new `SingleEraseRewriter` (that is owned
by the `ConversionPatternRewriterImpl`) if there is chance of
double-free. This rewriter ignores `eraseOp`/`eraseBlock` if the
op/block was already freed.
This is needed by PR#77665[1] that uses a P-register while restoring
Z-registers.
The reverse for SVE register restore in the epilogue was added to
guarantee performance, but further work was done to improve sve frame
restore and besides that the schedule also may change the order of the
restore, undoing the reverse restore.
This also fix the problem reported in (PR #79623) on Windows with
std::reverse and .base().
[1]https://github.com/llvm/llvm-project/pull/77665
This commit improves the block signature conversion API of the dialect
conversion.
There is the following comment in
`ArgConverter::applySignatureConversion`:
```
// If no arguments are being changed or added, there is nothing to do.
```
However, the implementation actually used to replace a block with a new
block even if the block argument types do not change (i.e., there is
"nothing to do"). This is fixed in this commit. The documentation of the
public `ConversionPatternRewriter` API is updated accordingly.
This commit also removes a check that used to *sometimes* skip a block
signature conversion if the block was already converted. This is not
consistent with the public `ConversionPatternRewriter` API; blocks
should always be converted, regardless of whether they were already
converted or not.
Block signature conversion also used to be silently skipped when the
specified block was detached. Instead of silently skipping, an assertion
is triggered. Attempting to convert a detached block (which is likely an
erased block) is invalid API usage.
See the comments added to the code for details on the inaccuracies that
have
now been fixed.
The patch adds tests that fail with the old implementation.
We need to iterate through the integers twice in isSimpleVIDSequence, so
instead of computing them twice just compute them once at the start.
This also replaces the individual checks that each element is constant
with a single call to BuildVectorSDNode::isConstant.
