llvm / llvm/llvm-project

[LICM] Preserve nsw when reassociating integer adds

Open
#220,897 0 comments 0 reactions 0 assignees View on GitHub
loopoptim missed-optimization
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.