[llvm] [SLSR] Defer removal of poison-generating annotations if not executed (PR #217093)

[via llvm-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-llvm-transforms

Author: Nick Riasanovsky (njriasan)

<details>
<summary>Changes</summary>

Defer removal of poison-generating annotations in SLSR until the selected rewrite is actually executed.

Previously, candidate discovery could remove annotations such as nuw and nsw even when the rewrite was later skipped by a debug counter. The pass now retains the annotations during candidate analysis and removes them only after committing to a rewrite.

The debug-counter test now verifies both behaviors:

- Skipped rewrites preserve poison-generating annotations.
- Executed rewrites remove annotations required for safe basis reuse.

---
Full diff: https://github.com/llvm/llvm-project/pull/217093.diff


2 Files Affected:

- (modified) llvm/lib/Transforms/Scalar/StraightLineStrengthReduce.cpp (+7-9) 
- (modified) llvm/test/Other/debugcounter-slsr.ll (+35-25) 


``````````diff
diff --git a/llvm/lib/Transforms/Scalar/StraightLineStrengthReduce.cpp b/llvm/lib/Transforms/Scalar/StraightLineStrengthReduce.cpp
index 8826127dd7ae3..deed6160b2056 100644
--- a/llvm/lib/Transforms/Scalar/StraightLineStrengthReduce.cpp
+++ b/llvm/lib/Transforms/Scalar/StraightLineStrengthReduce.cpp
@@ -217,8 +217,8 @@ class StraightLineStrengthReduce {
     // Points to (Y - X) that will be used to rewrite this candidate.
     Value *Delta = nullptr;
 
-    // List of instructions we need to drop poison generating annotations from.
-    // This is used so we can defer dropping until the candidate is evaluated.
+    // List of instructions whose poison-generating annotations must be dropped
+    // if this candidate is used as the basis of an executed rewrite.
     SmallVector<Instruction *> DropList;
 
     /// Cost model: Evaluate the computational efficiency of the candidate.
@@ -760,11 +760,6 @@ bool StraightLineStrengthReduce::candidatePredicate(Candidate *Basis,
       !C.isProfitableRewrite(*Delta, Candidate::IndexDelta))
     return false;
 
-  // If there is a Delta that we can reuse Basis to rewrite C, clean up
-  // previously collected poison generating instructions.
-  for (Instruction *I : Basis->DropList)
-    I->dropPoisonGeneratingAnnotations();
-
   // Record delta if none has been found yet, or the new delta is
   // a constant that is better than the existing delta.
   if (!C.Delta || isa<ConstantInt>(Delta)) {
@@ -1050,8 +1045,8 @@ void StraightLineStrengthReduce::allocateCandidatesAndFindBasis(
   RewriteCandidates[C.Ins].push_back(&Candidates.back());
   // Only add to the dict if this instruction is safe to reuse as a basis. By
   // doing this early we avoid calling canReuseInstruction repeatedly for the
-  // same instruction. The DropList is stored on the Candidate so
-  // candidatePredicate can drop the flags when a rewrite is being done.
+  // same instruction. The DropList is stored on the Candidate so the flags can
+  // be dropped only if this candidate is used by an executed rewrite.
   if (!EnablePoisonReuseGuard ||
       SE->canReuseInstruction(SE->getSCEV(I), I, Candidates.back().DropList)) {
     CandidateDict.add(Candidates.back());
@@ -1304,6 +1299,9 @@ void StraightLineStrengthReduce::rewriteCandidate(const Candidate &C) {
   assert(C.Delta && C.CandidateKind == Basis.CandidateKind &&
          C.hasValidDelta(Basis));
 
+  for (Instruction *I : Basis.DropList)
+    I->dropPoisonGeneratingAnnotations();
+
   IRBuilder<> Builder(C.Ins);
   Value *Bump = emitBump(Basis, C, Builder, DL);
   Value *Reduced = nullptr; // equivalent to but weaker than C.Ins
diff --git a/llvm/test/Other/debugcounter-slsr.ll b/llvm/test/Other/debugcounter-slsr.ll
index 0e24f493c3bc8..27abd033814b5 100644
--- a/llvm/test/Other/debugcounter-slsr.ll
+++ b/llvm/test/Other/debugcounter-slsr.ll
@@ -1,25 +1,35 @@
-; RUN: opt -passes=slsr -S -debug-counter=slsr-counter=1  < %s | FileCheck %s
-
-; Test that, with debug counters on, we will skip the first slsr opportunity.
-
-define void @stride_is_2s(i32 %b, i32 %s) {
-; CHECK-LABEL: @stride_is_2s(
-; CHECK-NEXT: %s2 = shl i32 %s, 1
-; CHECK-NEXT: %t1 = add i32 %b, %s2
-; CHECK-NEXT: call void @foo(i32 %t1)
-; CHECK-NEXT: %s4 = shl i32 %s, 2
-; CHECK-NEXT: %t2 = add i32 %b, %s4
-; CHECK-NEXT: call void @foo(i32 %t2)
-; CHECK-NEXT: ret void
-;
-  %s2 = shl i32 %s, 1
-  %t1 = add i32 %b, %s2
-  call void @foo(i32 %t1)
-  %s4 = shl i32 %s, 2
-  %t2 = add i32 %b, %s4
-  call void @foo(i32 %t2)
-  ret void
-}
-
-declare void @foo(i32)
-
+; RUN: opt -passes=slsr -S -debug-counter=slsr-counter=1 < %s | FileCheck %s --check-prefix=SKIP
+; RUN: opt -passes=slsr -S < %s | FileCheck %s --check-prefix=EXEC
+
+; Test that poison-generating annotations are dropped only when an SLSR
+; opportunity is executed.
+
+define void @stride_is_2s(i32 %b, i32 %s) {
+; SKIP-LABEL: @stride_is_2s(
+; SKIP-NEXT: %s2 = shl nuw nsw i32 %s, 1
+; SKIP-NEXT: %t1 = add nuw nsw i32 %b, %s2
+; SKIP-NEXT: call void @foo(i32 %t1)
+; SKIP-NEXT: %s4 = shl i32 %s, 2
+; SKIP-NEXT: %t2 = add i32 %b, %s4
+; SKIP-NEXT: call void @foo(i32 %t2)
+; SKIP-NEXT: ret void
+;
+; EXEC-LABEL: @stride_is_2s(
+; EXEC-NEXT: %s2 = shl i32 %s, 1
+; EXEC-NEXT: %t1 = add i32 %b, %s2
+; EXEC-NEXT: call void @foo(i32 %t1)
+; EXEC-NEXT: [[BUMP:%.*]] = shl i32 %s, 1
+; EXEC-NEXT: %t2 = add i32 %t1, [[BUMP]]
+; EXEC-NEXT: call void @foo(i32 %t2)
+; EXEC-NEXT: ret void
+;
+  %s2 = shl nuw nsw i32 %s, 1
+  %t1 = add nuw nsw i32 %b, %s2
+  call void @foo(i32 %t1)
+  %s4 = shl i32 %s, 2
+  %t2 = add i32 %b, %s4
+  call void @foo(i32 %t2)
+  ret void
+}
+
+declare void @foo(i32)

``````````

</details>


https://github.com/llvm/llvm-project/pull/217093