[SCEVExpander] Do not reuse disjoint or

[nikic]

SCEV treats "or disjoint" the same as "add nsw nuw". However, when expanding, we cannot generally replace an add SCEV node with an "or disjoint" instruction. Just dropping the poison flag is insufficient in this case, we would have to actually convert the or into an add.

This is a partial fix for #79861.

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

Author: Nikita Popov (nikic)

Changes

SCEV treats "or disjoint" the same as "add nsw nuw". However, when expanding, we cannot generally replace an add SCEV node with an "or disjoint" instruction. Just dropping the poison flag is insufficient in this case, we would have to actually convert the or into an add.

This is a partial fix for #79861.

---

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

2 Files Affected:

• (modified) llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp (+7)
• (modified) llvm/test/Transforms/IndVarSimplify/pr79861.ll (+3-2)

diff --git a/llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp b/llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp
index ed55a13072aaa..eea01825a72b2 100644
--- a/llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp
+++ b/llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp
@@ -1370,6 +1370,13 @@ canReuseInstruction(ScalarEvolution &SE, const SCEV *S, Instruction *I,
   if (programUndefinedIfPoison(I))
     return true;
 
+  // Disjoint or instructions are interpreted as adds by SCEV. However, we
+  // can't replace an arbitrary add with disjoint or, even if we drop the flag.
+  // We would need to convert the or into an add.
+  if (auto *PDI = dyn_cast<PossiblyDisjointInst>(I))
+    if (PDI->isDisjoint())
+      return false;
+
   // Otherwise, it is possible that I is more poisonous that S. Collect the
   // poison-contributors of S, and then check whether I has any additional
   // poison-contributors. Poison that is contributed through poison-generating
diff --git a/llvm/test/Transforms/IndVarSimplify/pr79861.ll b/llvm/test/Transforms/IndVarSimplify/pr79861.ll
index a8e2aa42a365c..7e267d04c94cc 100644
--- a/llvm/test/Transforms/IndVarSimplify/pr79861.ll
+++ b/llvm/test/Transforms/IndVarSimplify/pr79861.ll
@@ -75,14 +75,15 @@ define void @expander_or_disjoint(i64 %n) {
 ; CHECK-LABEL: define void @expander_or_disjoint(
 ; CHECK-SAME: i64 [[N:%.*]]) {
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    [[OR:%.*]] = or i64 [[N]], 1
+; CHECK-NEXT:    [[OR:%.*]] = or disjoint i64 [[N]], 1
+; CHECK-NEXT:    [[TMP0:%.*]] = add i64 [[N]], 1
 ; CHECK-NEXT:    br label [[LOOP:%.*]]
 ; CHECK:       loop:
 ; CHECK-NEXT:    [[IV:%.*]] = phi i64 [ 0, [[ENTRY:%.*]] ], [ [[IV_INC:%.*]], [[LOOP]] ]
 ; CHECK-NEXT:    [[IV_INC]] = add i64 [[IV]], 1
 ; CHECK-NEXT:    [[ADD:%.*]] = add i64 [[IV]], [[OR]]
 ; CHECK-NEXT:    call void @use(i64 [[ADD]])
-; CHECK-NEXT:    [[EXITCOND:%.*]] = icmp ne i64 [[IV_INC]], [[OR]]
+; CHECK-NEXT:    [[EXITCOND:%.*]] = icmp ne i64 [[IV_INC]], [[TMP0]]
 ; CHECK-NEXT:    br i1 [[EXITCOND]], label [[LOOP]], label [[EXIT:%.*]]
 ; CHECK:       exit:
 ; CHECK-NEXT:    ret void

[preames]

LGTM

Mostly to make sure I understand this properly, an alternate fix here would be to treat the or as needing it's poison generating flags dropped, and then mutate the or to an add in the same place we're dropping metadata right?

[nikic]

Mostly to make sure I understand this properly, an alternate fix here would be to treat the or as needing it's poison generating flags dropped, and then mutate the or to an add in the same place we're dropping metadata right?

Yes, exactly.

[fhahn]

LGTM, thanks!