Do not short circuit hoistIVInc when recomputation of poison flags is needed.

[resistor]

Summary

Fixes https://github.com/llvm/llvm-project/issues/59777

[nikic]

llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp

1029

We can't just fall through to the code below if RecomputePoisonFlags is set. This both does more than we need, and I believe it will produce broken IR in some cases, because it may end up moving the instruction down, making it no longer dominate other users.

I believe what you'd want to do is extract the poison flag handling code below and then call it in this short-circuit code path as well.

llvm/test/Transforms/IndVarSimplify/iv-poison.ll

4

Include a reference to PR59777 here (e.g. in a comment).

Instructions in tests should always be named, you can achieve this by running the test through -passes=instnamer.

[resistor]

Extract poison fixup logic to a helper and apply it in the short-circuit case without
going through the rest of the hoisting process.

[nikic]

llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp

1045

nit: Should not be on one line.

1068

Same

llvm/test/Transforms/IndVarSimplify/iv-poison.ll

4

Not addressed

[nikic]

(For the nits, otherwise fine)

[resistor]

Address review feedback.

[nikic]

LGTM. This will be a minor compile-time regression, but it's a correctness fix...

[resistor]

Fix improperly updated testcase.

llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp

if (InsertPtGuard->GetInsertPoint() == It)
       InsertPtGuard->SetInsertPoint(NewInsertPt);
 }
 
 /// hoistStep - Attempt to hoist a simple IV increment above InsertPos to make
 /// it available to other uses in this loop. Recursively hoist any operands,
 /// until we reach a value that dominates InsertPos.
 bool SCEVExpander::hoistIVInc(Instruction *IncV, Instruction *InsertPos,
                               bool RecomputePoisonFlags) {
-  if (SE.DT.dominates(IncV, InsertPos))
+  if (SE.DT.dominates(IncV, InsertPos) && !RecomputePoisonFlags)
       return true;
 
   // InsertPos must itself dominate IncV so that IncV's new position satisfies
   // its existing users.
   if (isa<PHINode>(InsertPos) ||
       !SE.DT.dominates(InsertPos->getParent(), IncV->getParent()))
     return false;
 
   if (!SE.LI.movementPreservesLCSSAForm(IncV, InsertPos))
     return false;
 
   // Check that the chain of IV operands leading back to Phi can be hoisted.
   SmallVector<Instruction*, 4> IVIncs;
   for(;;) {
     Instruction *Oper = getIVIncOperand(IncV, InsertPos, /*allowScale*/true);
     if (!Oper)
       return false;
     // IncV is safe to hoist.
     IVIncs.push_back(IncV);
     IncV = Oper;
     if (SE.DT.dominates(IncV, InsertPos))
       break;
   }
   for (Instruction *I : llvm::reverse(IVIncs)) {
     fixupInsertPoints(I);
     I->moveBefore(InsertPos);
     if (!RecomputePoisonFlags)
       continue;
     // Drop flags that are potentially inferred from old context and infer flags
     // in new context.
     I->dropPoisonGeneratingFlags();
     if (auto *OBO = dyn_cast<OverflowingBinaryOperator>(I))
       if (auto Flags = SE.getStrengthenedNoWrapFlagsFromBinOp(OBO)) {
         auto *BO = cast<BinaryOperator>(I);
         BO->setHasNoUnsignedWrap(
             ScalarEvolution::maskFlags(*Flags, SCEV::FlagNUW) == SCEV::FlagNUW);
         BO->setHasNoSignedWrap(
             ScalarEvolution::maskFlags(*Flags, SCEV::FlagNSW) == SCEV::FlagNSW);
       }
   }
   return true;
 }
 
 /// Determine if this cyclic phi is in a form that would have been generated by
 /// LSR. We don't care if the phi was actually expanded in this pass, as long
 /// as it is in a low-cost form, for example, no implied multiplication. This
 /// should match any patterns generated by getAddRecExprPHILiterally and

llvm/test/Transforms/IndVarSimplify/iv-poison.ll

+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt -passes=indvars -S < %s | FileCheck %s
+
+define i2 @iv_hoist_nsw_poison(i2 %0) {
+; CHECK-LABEL: @iv_hoist_nsw_poison(
+; CHECK-NEXT:    br label [[TMP2:%.*]]
+; CHECK:       2:
+; CHECK-NEXT:    [[DOT07:%.*]] = phi i2 [ 1, [[TMP1:%.*]] ], [ [[TMP3:%.*]], [[TMP2]] ]
+; CHECK-NEXT:    [[TMP3]] = add nuw i2 [[DOT07]], 1
+; CHECK-NEXT:    [[DOTNOT_NOT:%.*]] = icmp ult i2 1, [[TMP0:%.*]]
+; CHECK-NEXT:    br i1 [[DOTNOT_NOT]], label [[COMMON_RET:%.*]], label [[TMP2]]
+; CHECK:       common.ret:
+; CHECK-NEXT:    [[DOTLCSSA:%.*]] = phi i2 [ [[TMP3]], [[TMP2]] ]
+; CHECK-NEXT:    ret i2 [[DOTLCSSA]]
+;
+  br label %2
+
+2:                                                ; preds = %2, %1
+  %.07 = phi i2 [ 1, %1 ], [ %3, %2 ]
+  %.0 = phi i2 [ 1, %1 ], [ %4, %2 ]
+  %3 = add nsw i2 %.07, 1
+  %4 = add i2 %.0, 1
+  %.not.not = icmp ult i2 %.07, %0
+  br i1 %.not.not, label %common.ret, label %2
+
+common.ret:                                       ; preds = %2
+  ret i2 %4
+}