[LICM] Preserve nsw when reassociating integer adds
- Dominant language
- LLVM
- Stars
- 40.5k
- Forks
- 18.7k
- PR merge metrics
- PR metrics pending
Description
`hoistBOAssociation()` can rewrite
```text
(LV + C1) + C2 -> LV + (C1 + C2)
```
This lets LICM hoist `C1 + C2` out of the loop.
When both original adds have `nsw` but at least one lacks `nuw`, the new adds lose `nsw`.
This also happens when LLVM can prove that `C1 + C2` does not overflow.
Losing `nsw` can block another optimization later in the same LICM run.
## Reproducer
This example is reduced from [`llvm/test/Transforms/LoopUnroll/ephemeral.ll`](https://github.com/llvm/llvm-project/blob/7b58716d96c3ae4c0c4e6f72e29b16137bb6224b/llvm/test/Transforms/LoopUnroll/ephemeral.ll#L8-L41):
```llvm
declare void @llvm.assume(i1)
define i32 @test1(ptr %a) {
entry:
br label %for.body
for.body:
%iv = phi i64 [ 0, %entry ], [ %iv.next, %for.body ]
%sum = phi i32 [ 0, %entry ], [ %sum.next, %for.body ]
%arrayidx = getelementptr inbounds i32, ptr %a, i64 %iv
%value = load i32, ptr %arrayidx, align 4
%add1 = add nsw i32 %value, 2
%add2 = add nsw i32 %add1, 4
%add3 = add nsw i32 %add2, 4
%add4 = add nsw i32 %add3, 4
%add5 = add nsw i32 %add4, 4
%add6 = add nsw i32 %add5, 4
%add7 = add nsw i32 %add6, 4
%add8 = add nsw i32 %add7, 4
%add9 = add nsw i32 %add8, 4
%add10 = add nsw i32 %add9, 4
%condition = icmp sgt i32 %add10, -7
call void @llvm.assume(i1 %condition)
%sum.next = add nsw i32 %value, %sum
%iv.next = add i64 %iv, 1
%exit = icmp eq i64 %iv.next, 5
br i1 %exit, label %for.end, label %for.body
for.end:
ret i32 %sum.next
}
```
Run:
```shell
opt -passes='loop-mssa(licm)' -S ephemeral.ll
```
[Godbolt reproducer](https://godbolt.org/z/5nK47Tfcz)
The existing test runs only `loop-unroll`.
The command above runs LICM directly to show the problem.
Current LICM folds the constant chain but drops `nsw`:
```llvm
%add10.reass = add i32 %value, 38
%condition = icmp sgt i32 %add10.reass, -7
```
Without `nsw`, the [`hoistAdd()` match](https://github.com/llvm/llvm-project/blob/7b58716d96c3ae4c0c4e6f72e29b16137bb6224b/llvm/lib/Transforms/Scalar/LICM.cpp#L2582-L2584) fails.
If LICM preserves `nsw`, the same pass run removes the add:
```llvm
%condition = icmp sgt i32 %value, -45
```
## Why this is safe
The original `nsw` flags tell us that `LV + C1` and `(LV + C1) + C2` do not overflow in signed arithmetic.
If LLVM can prove the same for `C1 + C2`, the invariant add computes its exact value.
`LV + (C1 + C2)` then has the same representable result as the original expression, so both reassociated adds can keep `nsw`.
## Root cause
`hoistBOAssociation()` sets `AllKnownNonNegative` to false.
Therefore, `OverflowTracking::applyFlags()` preserves `nsw` only when both original adds also have `nuw`.
`hoistBOAssociation()` does not currently try a signed-overflow proof.
Relevant source at revision `7b58716d96c3`:
- [`hoistBOAssociation()`](https://github.com/llvm/llvm-project/blob/7b58716d96c3ae4c0c4e6f72e29b16137bb6224b/llvm/lib/Transforms/Scalar/LICM.cpp#L2855-L2920)
- [`OverflowTracking::applyFlags()`](https://github.com/llvm/llvm-project/blob/7b58716d96c3ae4c0c4e6f72e29b16137bb6224b/llvm/lib/Transforms/Utils/Local.cpp#L4063-L4070)
## Suggested fix
After merging the flags, check whether the new invariant add can overflow in signed arithmetic.
Use the preheader terminator as the query context because the new add is created in the preheader.
One possible implementation is:
```diff
@@ hoistBOAssociation(Instruction &I, Loop &L, ...) @@
} else {
OverflowTracking Flags;
Flags.AllKnownNonNegative = false;
Flags.AllKnownNonZero = false;
Flags.mergeFlags(*BO);
Flags.mergeFlags(*BO0);
+ // HasNSW remains true for opcodes without no-wrap flags.
+ if (Opcode == Instruction::Add && Flags.HasNSW && !Flags.HasNUW &&
+ computeOverflowForSignedAdd(
+ C1, C2,
+ SimplifyQuery(L.getHeader()->getDataLayout(), DT, AC,
+ Preheader->getTerminator())) ==
+ llvm::OverflowResult::NeverOverflows)
+ Flags.AllKnownNonNegative = true;
+
// If `Inv` was not constant-folded, a new Instruction has been created.
if (auto *I = dyn_cast(Inv))
Flags.applyFlags(*I);
Flags.applyFlags(*NewBO);
```
The proposed code uses `AllKnownNonNegative` to pass the no-overflow proof to `applyFlags()`, although the proof does not imply that the operands are non-negative.
This proposal covers scalar and vector integer adds.
Multiplication is out of scope because `applyFlags()` also requires `AllKnownNonZero` before preserving its flags.
If the proof fails, LICM keeps its current behavior and reassociates without `nsw`.
New LICM tests should cover scalar and vector additions where the proof succeeds, an invariant add that may wrap, and the existing `nuw nsw` case.
## Existing precedent
- [PR #140404](https://github.com/llvm/llvm-project/pull/140404) added `OverflowTracking` to `hoistBOAssociation()` and initialized `AllKnownNonNegative` and `AllKnownNonZero` to false "for now."
- [`hoistAdd()`](https://github.com/llvm/llvm-project/blob/7b58716d96c3ae4c0c4e6f72e29b16137bb6224b/llvm/lib/Transforms/Scalar/LICM.cpp#L2602-L2604) already requires a no-overflow proof for its own reassociation.
## Open question: should LICM skip when it cannot preserve nsw?
Skipping the reassociation would keep `nsw`.
[PR #151492](https://github.com/llvm/llvm-project/pull/151492) is precedent for rejecting a LICM reassociation when it could make later optimizations less effective.
However, `ReassociatePass` performs the same regrouping and drops `nsw` before the loop pass manager in the default `-O3` pipeline.
In an A/B over 25,237 of LLVM's own test inputs, skipping the reassociation suppressed two hoists, and every difference disappeared after `-O3`.
For that reason, this issue proposes only preserving `nsw` when it can be proved.
Whether LICM should skip the reassociation otherwise is a separate question.
Contributor guide
Research direction
Start in llvm/lib/Transforms/Scalar/LICM.cpp at hoistBOAssociation() and compare its flag handling with hoistAdd(); read OverflowTracking::applyFlags() in llvm/lib/Transforms/Utils/Local.cpp. Run opt -passes='loop-mssa(licm)' -S on the ephemeral.ll reproducer, then add LICM tests for scalar and vector additions, wrapping cases, and the existing nuw nsw case. Done means proven-safe reassociation preserves nsw while potentially wrapping cases retain current behavior.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- compilers
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 58/100