[pull] main from llvm:main - #1726
Merged
Merged
Conversation
A change landed that enabled SLP vectorization in more places, but it caused a 16% regression in 508.namd_r on the spacemit-k3: #214417 The root cause is that we're emitting two vfmv.f.s now which are generally slow due to the vector->scalar domain crossing: https://godbolt.org/z/dMo55zPMa Reciprocal throughput of vmv.x.s/vfmv.f.s is 6 times higher than vmv.s.x/vfmv.s.f on the spacemit-k3 and 2.5 higher on the P870 according to https://camel-cdr.github.io/rvv-bench-results/ This PR increases the cost of a vmv.x.s/vfmv.f.s from 1->2, which is just enough to prevent unprofitable SLP vectorization and fixes the regression on 508.namd_r. It also has the bonus side effect of preventing loop vectorization where we end up scalarizing libcalls, see llvm/test/Transforms/LoopVectorize/RISCV/tail-folding-call-intrinsics.ll Fixes #214417
These are copied from the X86 tests with some adjustments.
I missed a missing import from #214410, found this failing downstream
Sinking this intrinsic with different arguments requires the use of a phi node which violates verifier assumptions about the intrinsic. So we mark it as unable to have operands replaced with variables. Fixes #214565
This is a follow-up to a comment on #213128. addSectionDelta was only called in sites within if statements that would branch on whether or not split DWARF is enabled and then branching to either addSectionDelta or addSectionLabel. Instead we can just make addSectionLabel call addSectionDelta internally if we're in a DwoUnit to make things simpler. This is obviously NFC for most cases. For macro info, we were previously branching on useSplitDWARF which can differ, but only in the case we are emitting into the skeleton compile unit, which we should not be doing for macro info.
…tores (#213935) The custom truncating store selector always created a subregister copy matching the memory type. When storing an i16 value to i8 both types used GPR32, producing an invalid copy: ``` *** Bad machine code: Invalid subregister index for virtual register *** - function: truncstores - basic block: %bb.0 (0xb1a58e8738c0) - instruction: %2:gpr32 = COPY %val16.sub_32:gpr32 - operand 1: %val16.sub_32:gpr32 Register class GPR32 does not support subreg index sub_32 LLVM ERROR: Found 1 machine code errors. ``` This code path should only be used for FPRs. The existing RegBankSelect avoids this by inserting a G_ANYEXT from i16 to i32, allowing the imported pattern to match. It was exposed by the type-based RegBankSelect prototype in #199040. Assisted-by: codex
Reflect the class change on FreeBSD code. Fixes: de4f929 ("[lldb][x86] Remove GCC 6 workaround")
) Currently each vector instruction just has a cost of 1 but at higher LMULs this isn't accurate. Also it looks like the lowering might have changed since the cost was added, but for inserts we now use vslide1down instead of vslide1up, see the codegen in insertelt-int-rv32.ll. --------- Co-authored-by: Ryan Buchner <buchner.ryan@gmail.com>
Those parameters don't exist anymore.
AArch64 compare-and-branch instructions can usually be reversed by changing the condition and adjusting the immediate. At boundary values, that adjustment can underflow or overflow, leaving the branch non-reversible. When condition flags are dead at such a branch, split the compare branch into an explicit compare followed by a conditional branch. The new sequence lets branch fixup reverse the condition without relying on an out-of-range adjusted immediate. Teach the branch-fixing paths to use cached branch liveness information when deciding whether this expansion is legal. The liveness snapshot is built before branch relaxation/fixup and is safe for the current users: they only insert trampolines/stubs between existing CFG edges or invert branches without changing program semantics. Since expansion can grow the source basic block, update the local address accounting used by branch relaxation. Assisted-by: Codex
For lvalue APValues.
…r constants (#160083) Similarly to what it is being already done in GVN (fb632ed), make sure pointers equalities derived via PredicatedInfo may be propagated so long as pointer constants' provenance is taken into account, which may be non-existent. This is achieved by tracking provenance in ValueLatticeElement and restrain replacement to uses allowed by `canReplacePointersInUseIfEqual`. Fixes: #159565.
`PseudoCALLReg` models a returning call with an explicit link register. The machine outliner uses it for normal outlined calls that return through `x5`. Marking this pseudo as a barrier is inconsistent with call sites at the end of a block that falls through to a successor after the outlined function returns. MachineVerifier correctly diagnoses those blocks as malformed. Remove the barrier flag and add a MIR regression test that outlines a sequence at the end of a fall-through block. The test runs for RV32 and RV64 with `-verify-machineinstrs`. The attribute isBarrier from PseudoTLSDESCCall is removed as well for the similar reason and a MachineVerifier regression test is added for the TLSDESC call at the end of a fall-through block.
This is a follow up to #213010.
Fix a crash in the SPIR-V backend when an aggregate `extractvalue` result is used as the base of a later `insertvalue`. The failing pattern is: ```llvm %e = extractvalue [1 x [1 x i64]] %a, 0 %i = insertvalue [1 x i64] %e, i64 %x, 0 %r = insertvalue [1 x [1 x i64]] %a, [1 x i64] %i, 0 ``` `SPIRVPrepareFunctions` rewrites aggregate function arguments and returns to `i32` SPIR-V value IDs. The `llvm.spv.insertv` intrinsic also models its composite operand as an `i32` value ID. However, an intermediate aggregate `extractvalue` could still be rewritten to `llvm.spv.extractv` with an LLVM aggregate result type. Passing that aggregate-typed value to `llvm.spv.insertv` made `IRBuilder::CreateIntrinsic` create a call with a mismatched signature and assert. This patch mutates aggregate-result `llvm.spv.extractv` values that feed `insertvalue` to `i32`, while preserving the original aggregate type in the existing aggregate type metadata path. Selection then emits the expected SPIR-V: ```spirv %inner = OpCompositeExtract %Inner %outer 0 %new_inner = OpCompositeInsert %Inner %x %inner 0 %new_outer = OpCompositeInsert %Outer %new_inner %outer 0 ``` Co-authored-by: Codex <noreply@openai.com>
`scf.for` has provided induction variable bounds through
`ValueBoundsOpInterface` for a long time, but `affine.for` has no model
at all. `ValueBoundsConstraintSet` therefore cannot derive any bound for
an affine induction variable, not even `iv >= lowerBound`. Queries just
return "unknown", which is easy to miss because it looks the same as a
bound that genuinely cannot be proven.
Add a model for the induction variable. The lower bound of an
`affine.for` is the maximum over the results of its lower bound map and
the upper bound is the minimum over the results of its upper bound map,
so the induction variable is constrained by every individual result.
This follows how `AffineMinOpInterface` and `AffineMaxOpInterface` in
the same file align their maps with the constraint set; the shared logic
is factored into `alignBoundExpr`.
When both maps have exactly one result the step is taken into account as
well, following `scf::ForOpInterface`: the induction variable is always
a multiple of `step` away from the lower bound, so it never exceeds `lb
+ (tripCount - 1) * step`. That is tighter than `ub - 1` whenever the
trip count is not a multiple of the step, for `affine.for %i = 0 to 300
step 128` the induction variable only ever takes {0, 128, 256}, so the
bound is 256 rather than 299. As in `scf::ForOpInterface` this does not
replace the `iv < ub` constraint, since constraints multiplying two
constraint set dimensions aren't supported. The step is not applied when
either map has multiple results (`max`/`min` bounds), since no single
result can drive the arithmetic.
No bounds are inferred for `iter_args`.
Beyond the lit tests, the bound arithmetic was checked against an oracle
sweep: 6268 loops over an exhaustive `(lb, ub, step)` grid plus 4000
random cases, with the true induction variable range computed by
simulating each loop in Python rather than by asking the compiler.
Across 12536 queries there were 0 unsound bounds and 0 loose ones —
every bound is exactly `max(iv)` and `min(iv)`, so the model captures
every provable case rather than a conservative subset. Two controls:
reverting the change makes all 12536 queries unprovable, and mutating
the step arithmetic to be one step too tight makes the sweep report 5643
unsound bounds with counterexamples. The harness discriminates in both
directions.
No existing test needed updating: on its own this causes no `in_bounds`
folding in the vector dialect, because `isInBounds` still bails on any
non-constant index. Using these bounds there is a follow-up I'd like to
send once this lands.
Assisted-by: Claude
…red file (#213427) Instead of copying the code around we can simply share a file that actually implements operator new/delete and include that file in the two places that need the definitions.
…,a),binop(y,b)) fold (#214285) A similar fold is cost-driven in VectorCombine, but this InstCombine fold was then undoing the decision Seems to an old fold added by @rotateright (#32373) before we created VectorCombine to do this properly Fixes #205707
Use cmake_format.py from #213102 on all CMakeLists.txt files in libc/ Assisted-by: Automated tooling, human reviewed.
Added getpwent, setpwent, and endpwent functions using the internal pwd_utils line parser. * Implemented getpwent.cpp, setpwent.cpp, and endpwent.cpp entrypoints * Added database iteration state and helpers (open, close, read_next) under namespace passwd in pwd_utils * Registered entrypoints in config/linux/*/entrypoints.txt * Added CMake target guards for File and platform_file to support overlay builds and cross-platform targets * Added unit tests in libc/test/src/pwd/getpwent_test.cpp Assisted-by: Automated tooling, human reviewed.
…214436) PTX ISA 9.4 adds the im2col_no_offs::w mode to shared-to-global tensor copy and reduction instructions for Rubin family targets. This change adds the corresponding NVVM intrinsics and NVPTX lowering.
…#212799) This patch restricts the generation of new atomic instructions added under FEAT_LSFE to functions where no-trapping-math flag is set and strictfp attribute is not set. This is done to preserve the exception behaviour of the original codegen.
#211569) This commit improves block scaled tensor type verification to provide a specific reason about why type verification failed. Previously the error message was a very generic "must be tosa-conformant tensor of number values". This commit restores previous functionality that was reverted by #207995, but uses the ODS string interpolation mechanism instead to prevent incompatibility with PDLL.
…ounts (#205102) SCEV can compute a small maximum trip count for a loop even when it cannot compute the exact backedge-taken count. This is typical of data-dependent exits, such as the varint-length loops in sqlite3: for (i = 1; (v >>= 7) != 0; i++) ; Unrolling to the maximum trip count turns the loop into a chain of copies containing an exit test for every possible iteration. This increases code size and the number of static branches without knowing whether enough iterations usually execute to make unrolling profitable. These loops were not unrolled before #197292. That change improved SCEV's maximum backedge-taken count for shift recurrences, reducing the maximum trip count of these loops to 6. This made them eligible for AArch64 upper-bound unrolling and caused an approximately 3% regression in sqlite3 from SPEC CPU 2026. For AArch64 loops with a single exiting block that are not MaxOrZero and whose exact backedge-taken count SCEV cannot compute, lower MaxUpperBound to 5 and disable runtime unrolling. The lower bound still allows smaller loops to be unrolled. Runtime unrolling is also disabled because it would clamp its unroll count to the known maximum and produce the same complete unroll.
…ed_div. (#213903) This follows the same idiom used by other masked operations to prevent poor type legalisation of boolean vectors.
In future it may be generating things other than flags. Functionality is the same, but the interface changes to use RegisterType.
…64` library location (#211587) On non-standard ROCm installations, `libamdhip64.so` may be under `/lib64` instead of `/lib`. To acomodate for these, if `/lib/libamdhip64.so` does not exists and `/lib64` does, use the later. On windows we check for `amdhip64.lib`. If both exist `/lib` is preferred. By default we conservatively use `/lib`. Related to LCOMPILER-2495.
#213902) Replace the two-operand boolean form !{!"llvm.loop.vectorize.predicate.enable", i1 0/1} with a single-operand enable/disable pair: !{!"llvm.loop.vectorize.predicate.enable"} !{!"llvm.loop.vectorize.predicate.disable"} The Verifier rejects the two-operand form, AutoUpgrade rewrites old bitcode, and the readers and producers in LLVM, Clang and MLIR are updated. Please refer to RFC: https://discourse.llvm.org/t/rfc-enforce-single-operand-format-for-all-enable-metadata-nodes/90571/ Assisted by AI
Following on from PR #212198, this patch also applies the same urem vplan folds to the vector preheader, which leads to a lot of test changes that are mostly mechanical. As a by-product it also fixes a compile-time regression introduced by #212198 due to the expensive `getVectorPreheader` function. In future we can do further folds similar to instcombine, since `(X - (X & Y)) --> (X & ~Y)` and we know that `urem(X, PowerOf2Y) -> X & log2(PowerOf2Y)`. However, the root matching pattern in this case needs to be a subtract.
## Summary
This patch fixes a DWARF debug-info emission bug where the pending
subregister mask state used for one `DW_OP_LLVM_arg` could leak into a
following full-register `DW_OP_LLVM_arg` in the same `DIArgList`
expression.
For example, when an `i32` value is described through the x86-64
super-register `RDI`, LLVM correctly emits:
```text
DW_OP_breg5 RDI+0
DW_OP_constu 0xffffffff
DW_OP_and
```
However, before this patch, the subregister state remained live after
emitting that mask. If the next `DW_OP_LLVM_arg` used a full 64-bit
register such as `RSI`, LLVM could incorrectly emit the same 32-bit mask
for it too:
```text
DW_OP_breg4 RSI+0
DW_OP_constu 0xffffffff
DW_OP_and
```
## Reproducer
Source code:
```c
#include <stdint.h>
volatile int sinki;
volatile uint64_t sinku64;
__attribute__((noinline))
int test(int32_t a, uint64_t b) {
int x = a == b;
sinki += a;
sinku64 += b;
return 0;
}
int main(void) {
return test(-1, UINT64_MAX);
}
```
Compile with: `clang -O2 -g -gdwarf-5 repro.c -o repro`
DebugInfo in IR for variable `x` (still correct at this point):
```
#dbg_value(!DIArgList(i32 %a, i64 %b), !x,
!DIExpression(DW_OP_LLVM_arg, 0,
DW_OP_LLVM_convert, 32, DW_ATE_signed,
DW_OP_LLVM_convert, 64, DW_ATE_signed,
DW_OP_LLVM_arg, 1,
DW_OP_eq,
DW_OP_LLVM_convert, 1, DW_ATE_unsigned,
DW_OP_LLVM_convert, 32, DW_ATE_unsigned,
DW_OP_stack_value), !loc)
```
Emitted DWARF Expression in binary:
```text
# get value of a
DW_OP_breg5 RDI+0
DW_OP_constu 0xffffffff
DW_OP_and
DW_OP_convert <signed_32>
DW_OP_convert <signed_64>
# get value of b (wrong)
DW_OP_breg4 RSI+0
DW_OP_constu 0xffffffff
DW_OP_and
# comparison
DW_OP_eq
DW_OP_stack_value
```
With the buggy DWARF expression, LLDB can evaluate `x` as `0`, even
though the source-level value is `1`.
## Fix
`DwarfExpression::maskSubRegister()` consumes the pending subregister
description when it emits the mask. This patch clears that state inside
`maskSubRegister()` after emitting the shift/mask operations, preventing
it from affecting later operands in the same expression.
A regression test (`test/DebugInfo/X86/diarglist-subregister-mask.ll`)
was added to check that the first subregister operand is masked, while
the following full-register operand is not.
## Validation
Locally verified with:
```
llvm-lit -sv llvm/test/DebugInfo/X86/diarglist-subregister-mask.ll
ninja -C build check-llvm
ninja -C build check-lldb
```
Both `check-llvm` and `check-lldb` completed with no unexpected
failures.
) My recent commit 91dfa26 moved this code here from a different file. Now that it has no menaningful git history (and it is blamed to me) let's get rid of the lowercase names.
InstCombine and(cmphs(pg, ConstA, A), cmphs(pg, A, ConstB)) -> cmphs(pg, ConstA - ConstB, sub(pg, A, ConstB)) Also emit zero when ConstB > ConstA
…invalidation (NFC) (#214694) A use-after-free issue has been addressed by avoiding a dangling `ValueState` reference in `visitGetElementPtrInst`, that could arise from `try_emplace` DenseMap reallocations in `getValueState`. ASan buildbots were previously failing.
Swig 4.5.0 released today and adds support for customizable automatic type annotations. We have to configure the `pytyping` typemap for all cases where we change the default Swig behavior (i.e. for all `in`/`out`/`argout` typemaps). This is what I did in this PR. In two cases, we can't properly annotate: - For all `argout` typemaps, Swig includes the return type of the function. So if we'd return a `typing.List[int]`, Swig assumes we're using `Swig_AppendOutput`, so it annotates the return type with `typing.List[typing.Union[ReturnType, typing.List[int]]]`. - Related to that, the annotations for `INPUT`/`OUTPUT`/`INOUT` are wrong, because the `pytyping` typemap doesn't know about them. There, I disabled the annotations. Furthermore, overloads are not shown. The issues are known, and I have fixes for them, but they will only land in Swig 4.6. It's a bit unfortunate that this happens after 23.x, but better late than never. In the future, we could use https://github.com/tox-dev/sphinx-autodoc-typehints to show the type hints in the documentation. One downside is that without support for overloads, some methods only show `*args`. You can see the current `__init__.py` in https://gist.github.com/Nerixyz/2bf230147f933e3943a87457bf036bb0. Towards #79043.
Mirrors the existing ShapeType / ShapeShiftType converters: a `!fir.shift<N>` becomes `!llvm.struct<(i64 x N)>`. Co-Authored-By: Claude
`allocate` is an OpenMP 5.0 clause, but 58 of the 62 directives that
allow it declare it as
bare `VersionedClause<OMPC_Allocate>`. The default in `DirectiveBase.td`
is `min = 1`:
```
class VersionedClause<Clause c, int min = 1, int max = 0x7FFFFFFF> : Versioned<min, max>
```
so those 58 accept the clause at every version. Only four are gated
today: `do`, `taskgroup`
and `parallel do` at 50, and `scope` at 52.
This is reachable in practice because flang defaults to OpenMP 3.1
(`newestFullySupported = 31`, `CompilerInvocation.cpp`). An invocation
with no
`-fopenmp-version=` lands in the ungated range, semantics accepts the
clause, and
`ConstructDecomposition` then correctly refuses to decompose it.
Lowering consumes the empty
result and crashes:
```
$ flang -fc1 -emit-hlfir -fopenmp repro.f90
Segmentation fault
```
In an assertions build it is caught at `Decomposer.cpp:85`:
```
Assertion `!decompose.output.empty() && "Construct decomposition failed"' failed.
```
With the gate, that becomes a diagnostic:
```
error: ALLOCATE clause is not allowed on TARGET TEAMS DISTRIBUTE PARALLEL DO directive
in OpenMP v3.1, try -fopenmp-version=50
```
Reported as #211430.
### Scope
This fixes reachability, not the underlying memory error, and should not
be read as a complete
fix for #211430. Consuming a failed decomposition reads uninitialized
memory, which is why the
crash is intermittent — measured on a release build at a pinned
`-fopenmp-version=31`, 40 trials
each:
| | segfaults |
|---|---|
| ASLR on | 25/40 |
| ASLR off (`setarch -R`) | 0/40 |
Any other clause/directive combination that makes decomposition return
empty will hit the same
path. Making that path diagnose and bail rather than fall through is a
separate change.
I also left the gate uniform at 50 rather than tightening per directive.
`allocators` is a 5.2
construct, so by the precedent `scope` sets its clause could be 52; that
argument applies to
several directives here and depends on each one's introduction version,
so it seemed better kept
out of a mechanical change.
### Tests
Nine tests exercised `allocate` through bare `%openmp_flags` (i.e.
`-fopenmp` with no version) and
so depended on the missing gate. Seven get `-fopenmp-version=50`. The
two `allocators` tests get
52, which is what their own `! OpenMP Version 5.2` header comments
already claim they are testing.
### Verification
| suite | tests | failed |
|---|---|---|
| `check-flang` | 4808 | 0 |
| `mlir/test/Dialect/OpenMP`, `mlir/test/Target/LLVMIR`,
`llvm/test/Frontend` | 425 | 0 |
| `clang/test/OpenMP` | 1594 | 0 |
clang version-checks `allocate` in its own semantic analysis rather than
through this table, but
it shares `OMP.td`, so I ran it to confirm.
---
Parts of this change were written or audited with Claude Code. I have
reviewed all of it and take
full responsibility for the contribution. See
`llvm/docs/AIToolPolicy.md`.
) The OpenMP specification includes a restriction for registering both EMI and non-EMI callbacks (e.g. `ompt_callback_target`, OpenMP v5.2, p. 503, l. 18-20): These callbacks must not be registered at the same time. However, this was not respected in the runtime. A tool was able to register both, receiving `ompt_set_always` for each `ompt_set_callback` call. When an event associated with the EMI or non-EMI callbacks was dispatched and both callbacks were registered, only the EMI version was dispatched. To align the runtime behavior with the OpenMP specification, return `ompt_set_error` when a tool tries to register both the EMI and non-EMI variant for a callback. With this, only the tool's first registration is used. Update the offload `veccopy_disallow_both.c` test, which already tested the prior, non-compliant, behavior. Closes #212959 Signed-off-by: Jan André Reuter <jan@zyten.de>
#213915) The previously used AArch64ISD::[SU]ADDW[BT] became unused after this change, so I've removed them.
Use stable fuchsia.dev URLs without `?hl=en` and add the missing reference on `fuchsia-temporary-objects`. Fixes #62334
…10351) This PR implements constant folding for `ISD::PARTIAL_REDUCE_SMLA`, `ISD::PARTIAL_REDUCE_UMLA`, and `ISD::PARTIAL_REDUCE_SUMLA` when their operands are constant `BUILD_VECTOR`s. The fold computes the partial reduction using `APInt` arithmetic and returns a folded `BUILD_VECTOR`. Input constants are truncated to their logical element width before being sign- or zero-extended as required by each opcode. Input lane `I` is accumulated into result lane `I % NumAccElts`, matching `TargetLowering::expandPartialReduceMLA`. The reduction order is deliberately unspecified, so this mapping is a valid refinement. Poison is propagated only to the affected result lanes. Folding is skipped for undef and opaque constants. Unsupported operand forms return early because partial-reduce nodes have no scalar form and must not fall through to generic per-element folding. After type legalization, when the accumulator element type is not a legal scalar type, the folded constants are created in the promoted legal type (matching the generic per-lane folding path), and the fold bails out if legalization would narrow the element type. The RISC-V test precommits the existing codegen for the basic constant fold. Additional IR tests cover accumulator-width wraparound, poison in the input and accumulator, and safe handling of scalable splats. Direct SelectionDAG unit tests are retained only for node forms that cannot be reliably or acceptably tested through IR: the SMLA/UMLA/SUMLA signedness matrix with promoted narrow inputs, promotion of the folded result to a legal scalar type after type legalization, poison in the implicit multiplier operand, undef and opaque elements, and partially constant no-fold cases. Fixes #209191
…nd into sshll/ushll. (#213989) Add PreprocessISelDAG logic to undo mul-of-shl to shl-of-mul canonicalisation when either operand is sign/zero extended because the extension can be folded into the shift.
This removes the XFAILs for the value API tests. - For `lldb/test/API/python_api/value/TestValueAPI.py`, we need to pass `/debug:symtab` to the linker to get a symbol table. Symbols are "hidden" by default. PDB works around this in the publics stream. For DWARF, we need `/debug:symtab`. An alternative is to use `/debug:dwarf` which also enables this. - For `lldb/test/API/python_api/value/change_values/TestChangeValueAPI.py` there were two issues. First, the stdout wasn't available when we stopped at a breakpoint. Adding a `fflush(stdout)` fixes this. Secondly, the breakpoint was created where the inputs to `printf` were already evaluated. I added another assignment where we set the breakpoint. The added `fflush` might also help with this being flakey on Linux (#26026). Closes #25146
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
20 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )