This PR proposes an update to the coding standards document to make
explicit that we do not want unnecessary formatting changes to TD files.
This is in response to this merged PR (#199346), which lead to this RFC
(https://discourse.llvm.org/t/80-column-limit-for-td-files/90950/).
---------
Co-authored-by: Vlad Serebrennikov <serebrennikov.vladislav@gmail.com>
The `valuesToReload` handling in our `RunCleanupScope::forceCleanup()`
function was not taking into account cleanup scopes for deferred
conditional cleanups that get created when we call
`forceDeactivation()`. This was leading to a CIR verification error in
cases where a deferred cleanup was used in an expression that returns a
value.
This change adds code to spill values ahead of the `forceDeactivation()`
call when we see that there are cleanups on the deferred stack.
Assisted-by: Cursor / claude-opus-4.7
Add a pre-aggregated profile output format (`--profile-format=preagg`)
so perf.data can be pre-parsed/aggregated and used as input with -pa.
Supports branch (T traces) and basic samples (S records).
Currently only covers main binary, can be extended to cover multi-DSO.
Test Plan: Updated perf_test.test, added perf_brstack.test
Reviewers:
yota9, ayermolo, yozhu, maksfb, yavtuk, paschalis-mpeis, rafaelauler
Reviewed By: paschalis-mpeis
Pull Request: https://github.com/llvm/llvm-project/pull/199465
Buffer atomic operations were failing LLVM validation because they
weren't declared as having a result (the old value). This commit updates
those operations to fix the error. (Note: if you don't need the result,
you just don't use it, and the compiler backend emits an atomic that
doesn't return the old value.)
LLDB-DAP has a problem with sanitizers in GCC. When we stop in
sanitizer's code, lldb-dap sends stack frames with path (sanitizer's
build dir path) that doesn't exist on machine. It leads to problems in
VS Code UI (see issue below).
Fixes#184789
Avoids wasteful SDValue creation if the shuffle matching fails, handles
any i64 legalisation, avoid issues with later folds not recognising fp
'allones' masks and makes it easier to add UNDEF element handling in the
future.
Auto loops are analyzed by the compiler in later compilation stages to
determine whether they can be parallelized. These loops may carry
parallelism levels (this does not guarantee that they are parallelizable,
compiler should still analyze them). However, if the loop is parallelized,
the parallelism levels specified in the source should be respected. This
change attaches the parallelism level to auto loops, which enables their
propagation through next compilation steps.
The PR adds support to define fallbacks for DLWRAP routines that are not
found when loading the library.
It implements a fallback for
zeCommandListAppendLaunchKernelWithArguments introduced in #194333 which
might not be available in older drivers.
The gpu-to-llvm conversion patterns for gpu.alloc and gpu.dealloc
previously required async tokens for non-host shared operations. This
prevented lowering synchronous device memory allocation and deallocation
to runtime calls.
Changes:
- gpu.alloc: drop the isAsyncWithOneDependency guard for non-shared
allocs. Cap the number of async dependencies at one to preserve the
prior single-dependency invariant. Cast the runtime-returned pointer to
the memref's address space when they differ, so the descriptor's pointer
slots type-check for memref<..., N>.
- gpu.dealloc: drop the async requirement entirely. Use a null stream
when no async dependencies are present. Use eraseOp instead of replaceOp
for sync deallocs (which have no results). Cap the number of async
dependencies at one.
- Add a unit test for the synchronous alloc/dealloc lowering and a test
that more than one async dependency leaves the op unconverted.
- Update lower-launch-func-bare-ptr.mlir, which previously asserted
gpu.alloc survived the conversion (because the pattern bailed for sync
non-shared allocs); it now asserts the full lowering through
mgpuMemAlloc + addrspacecast.
Assisted-by: Claude
…olution
After #198346, alpha.unix.cstring.UninitializedRead reports a false
positive when a pointer into a fully-initialized const multidimensional
array is advanced past an inner dimension boundary and used as a source
argument to memcpy. The root cause is in
`convertOffsetsFromSvalToUnsigneds` in RegionStore, which returned
UndefinedVal for any element index exceeding its sub-array extent,
conflating pointer arithmetic legality with memory initializedness.
This patch separates the two concerns. The RegionStore now normalizes
indices that overflow an inner dimension by carrying into the outer
dimension via divmod, the same way `arr[0][5]` in `int arr[4][3]`
denotes the same memory as `arr[1][2]`. UndefinedVal is returned only
when the computed flat offset exceeds the total array allocation.
Whether cross-subobject pointer arithmetic constitutes undefined
behavior per C/C++ standards is a separate concern for individual
checkers to diagnose. No existing checker flags sub-array boundary
crossing as UB, verified both before and after #198346.
Fixes#199271
…rone-unused-return-value
The cert-err33-c alias did not override CheckedReturnTypes, causing it
to inherit the default from bugprone-unused-return-value. This made it
flag any function returning std::error_code, std::expected, etc. That is
outside the scope of CERT ERR33-C (a fixed list of C standard library
functions).
Set CheckedReturnTypes to empty so the alias only checks its intended
function list.
---------
Co-authored-by: EugeneZelenko <eugene.zelenko@gmail.com>
Enables lowering/decomposing `linalg.pack` ops with dynamic inner tiles
to a sequence of `tensor.pad` -> `tensor.expand_shape` ->
`linalg.transpose` ops.
---------
Signed-off-by: Ege Beysel <beyselege@gmail.com>
This PR introduces a major refactor on how L0 queues are used in the
plugin as the current design is too tied to OpenMP behavior. There are
two major changes:
* We no longer have a per-thread queue cache as this resulted in a
single logical queue backed up by multiple L0 queues. We now have a per
device cache which should have a similar level of reuse performance.
* The AsyncQueueTy type has been largely extended to hide the logic of
the different queues types (which are now subclasses of AsyncQueueTy).
This has greatly simplified the L0Device implementation.
As part of this refactor a number of other changes happened:
* Copy command lists were removed in favor of the
ZE_COMMAND_QUEUE_FLAG_COPY_OFFLOAD_HINT driver hint.
* Support for inorder queues was added (can be selected using
LIBOMPTARGET_LEVEL_ZERO_COMMAND_MODE=inorder).
* Sync queues now use inorder queues.
* Queue operations conditionally only use events when necessary (OMPT,
profiling or queue logic).
* MemFill now support asynchronous operations
There are a few more things that still need to be adjusted and cleanup
but as this PR is already big enough as it is, I'll prepare a follow-up
to fix them.
Avoids wasteful SDValue creation if the shuffle matching fails, handles
any i64 legalisation and makes it easier to add UNDEF element handling
in the future.
We had a bug that was causing any lifetime-extended cleanups that
occurred within a full-expression cleanup scope to be emitted
prematurely when the expression also required deferred conditional
cleanups. This was, in some cases, causing a dangling reference to
temporaries that had already been destructed. Luckily, it was also
causing us to not emit a return at the end of the function in one case,
leading the verifier to draw attention to this problem.
This change introduces new functions in RunCleanupsScope to allow
"ordinary" EH stack cleanups to be force-emitted separately from
lifetime-extended cleanups. Classic codegen doesn't need this capability
because it handles deferred conditional cleanups very differently than
CIR due to its flat/branching approach.
The testing for this fix did uncover a significant issue wherein CIR is
calling destructors in the wrong order even after the fix in this PR.
However, that's a pre-existing issue that will require changes beyond
the scope of this fix, so I'll handle it in a follow-up.
Assisted-by: Cursor / claude-opus-4.7
This is a small correctness fix for foldShuffleChainsToReduce. For odd /
non-power-of-2 vector sizes the parity-mask scheme duplicates a lane,
which is only sound when the reduction op is idempotent. For
non-idempotent ops (e.g. add, xor) the duplicated lane changes the
result, so I track a HasLaneDuplication flag and bail out of the fold in
that case. Tests cover non-foldable add/xor and a still-foldable
idempotent smax.
Make sure the header phis in various tests are actually used, to make
them more robust w.r.t. to future simplification changes. Those dead
phis would be cleaned up before LV in the regular pipeline.
The langref says:
> The 'fptosi' instruction converts its floating-point operand into the
> nearest (rounding towards zero) signed integer value. If the value
> cannot fit in ty2, the result is a poison value.
Previously `fptosi to i1` and `fptoui to i1` were lowered as `x == 0.0`,
which is clearly incorrect.
Because the conversion truncates toward zero, the only results that are
not poison are:
- 0 and -1 for the signed case, and
- 0 and 1 for the unsigned case.
So the i1 result is fully determined by a single fp compare:
- `fptosi x to i1` == `x <= -1.0`
- `fptoui x to i1` == `x >= 1.0`
with any value being acceptable for the poison (out-of-range) inputs.
The current annotation markers can conflict with several language
constructs. Notably, `[[ ]]` and `^` collide with C++ attributes (e.g.,
`[[nodiscard]]`), the C++26 reflection operator (`^^int`), and
Objective-C blocks (`void (^foo)(void)`). Similarly, `$` can conflict
with identifiers that also use `$` with `-fdollars-in-identifiers`, as
well as with C++26 code that uses `$` as raw-string delimiters or as a
preprocessing token
([P2558R2](https://www.open-std.org/JTC1/SC22/WG21/docs/papers/2023/p2558r2.html)).
Because the markers are currently hardcoded in `llvm::Annotations`,
existing workarounds have to rely on digraphs or macro substitution via
implicit `#define`s. These approaches reduce readability and make tests
cumbersome to write. This PR alleviates these issues by adding support
for custom markers. It adds an overloaded `Annotations` constructor that
accepts a new `Annotations::Markers` struct. For example, to use `~` for
point, `@` for name, and `{{`/`}}` for range:
```cpp
Annotations Example(R"cpp(
@name(payload){{[[nodiscard]] int foo(int x);}}~
)cpp", {"~", "@", "{{", "}}"});
```
Alternatively, we can use setters to customize the markers individually:
```cpp
Annotations Example(R"cpp(
@name(payload){{[[nodiscard]] int foo(int x);}}~
)cpp", Annotations::Markers().setPoint("~")
.setName("@@")
.setRangeBegin("{{")
.setRangeEnd("}}"));
```
Using longer markers:
```cpp
Annotations Example(R"cpp(
$$name(payload)[[[[[nodiscard]] int foo(int x);]]]^^
)cpp", {"^^", "$$", "[[[", "]]]"});
```
Using multi-byte characters:
```cpp
Annotations Example(R"cpp(
🏷️name(payload)👉[[nodiscard]] int foo(int x);👈🎯
)cpp", {"🎯", "🏷️", "👉", "👈"});
```
The existing single-argument constructor delegates to the new overload,
preserving backward compatibility.
PS: The original code has a FIXME comment mentioning alternative
approaches, such as escaping and changing the default syntax. While
valid, escaping would increase visual noise, and changing the default
syntax would break existing tests. The approach proposed here provides
the flexibility to choose a syntax that is clean for a specific context
and is backward compatible. See https://reviews.llvm.org/D59814 for
earlier discussion about these alternative design choices.
Widening half/bfloat printf varargs via bitcast+sext corrupted the FP
bit pattern for negative values
Extend by value-preserving fpext to float instead
Fixes#199407
Remove ParamIdx from the `USE_DEFAULT_EQUALITY` default-comparison path
and add an explicit `equalAttrArgs<ParamIdx>` specialization that safely
handles invalid (unset) values.
Now in `CFGFullExprCleanup` we also store a cleanup expressions to be
able to get an accurate location where destruction happened.
This helps user understand lifetime semantics of objects better.
Closes#195503
This patch
* Supports parallel reductions for min/max operations in LoopUnroller.
* Adds relevant test (including intrinsics).
* Renames flag -unroll-add-parallel-reduction to
-unroll-parallel-reduction.
* Relaxes check in IVDescriptors.cpp (`getMinMaxRecurrence`) to handle
out-of-loop uses.
Planning to take support for vector types in the next patch.
Linear iteration variables were being treated as private. This fixes
one of the issues reported in #170784.
The regressions in the OpenMP V&V and Fujitsu testsuites happened
because the users iterator was apparently becoming invalid, after one of
its uses was replaced. This was fixed by making a copy of the list of
users.
This is related to #200335. I would like to nominate Ehsan as a
maintainer for DependenceAnalysis as I am aware he expressed interest in
that. I am happy that Ryotaro became a maintainer, and if we get one
more maintainer with Ehsan, that is a really good sign of a healthy loop
optimisation community; I think this is a good thing, and support this.
---------
Co-authored-by: Ehsan Amiri <ehsan.amiri@huawei.com>
This has [already bitten us
before](https://github.com/llvm/llvm-project/pull/71641) with
BlockAddresses, but another case popped up recently: under complex
conditions, LEAs in the three-operand case with symbolic displacements
could be miscompiled due to `MO_MCSymbol` not being handled in a similar
way. To avoid other issues in the future, just be more conservative
about the symbol type and only return false if we know for a fact that
the offset is zero.
Fixes#200707
PR #200111 stops compiling InstrProfilingPlatformROCm.cpp (which defines
the
HIP GPU helper __llvm_profile_hip_collect_device_data) in profile-only
builds.
But the compile define -DCOMPILER_RT_BUILD_PROFILE_ROCM=1 was still
added
whenever the COMPILER_RT_BUILD_PROFILE_ROCM option was on (the default),
so
InstrProfilingFile.c still referenced the helper from
__llvm_profile_write_file
even though it was never built.
On ELF the declaration is weak, so the undefined symbol folds to null
and the
address-guarded call is skipped. COFF/Windows has no such fallback:
error LNK2019: unresolved external symbol
__llvm_profile_hip_collect_device_data referenced in function
__llvm_profile_write_file
Add the define only when PROFILE_HAS_HIP_INTERCEPTOR is true, i.e. the
same
condition that keeps InstrProfilingPlatformROCm.cpp in the archive, so
the
macro is defined iff the helper is actually compiled in.
Reported by zmodem:
https://github.com/llvm/llvm-project/pull/200111#issuecomment-4593893230
Without this patch event listener registration was skipped, as a result
`Modules` view in UI was not displayed in case of launching target via
`launchCommands` or `attachCommands`.
