[LoopInstSimplify] Ignore users in unreachable blocks. PR55072

[mkazantsev]

Summary

Logic in this pass assumes that all users of loop instructions are
either in the same loop or are LCSSA Phis. In fact, there can also
be users in unreachable blocks that currently break assertions.
Such users don't need to go to the next round of simplifications.

[fhahn]

LGTM, thanks! I think it would be good to remove the branch on undef, as per inline comment and possible remove some of blocks that seem unneeded.

llvm/test/Transforms/LoopInstSimplify/pr55072.ll

4–5

nit: the test shouldn't be X86 specific, can this be removed?

14–15

Avoiding switching on undef might make the test less prone to unnecessary changes in case loop-instsimplify becomes more powerful in the future.

Branch/switch on undef is UB and if the source has UB this will mean verification with tools like Alive2 will always succeed, regardless of what the result of the transform is.

14–15

Those branches look like an artifact of llvm-reduce, where it fails to remove them. Are they needed?

[mkazantsev]

llvm/test/Transforms/LoopInstSimplify/pr55072.ll

4–5

I guess so. Will update the test before commiting changes.

[mkazantsev]

llvm/test/Transforms/LoopInstSimplify/pr55072.ll

14–15

https://github.com/llvm/llvm-project/commit/ab17873ee696dfbd6e7b10c5f78192f3990db970

[fhahn]

llvm/test/Transforms/LoopInstSimplify/pr55072.ll

14–15

Thanks!

llvm/lib/Transforms/Scalar/LoopInstSimplify.cpp

for (BasicBlock *BB : RPOT) {
         Value *V = SimplifyInstruction(&I, SQ.getWithInstruction(&I));
         if (!V || !LI.replacementPreservesLCSSAForm(&I, V))
           continue;
 
         for (Use &U : llvm::make_early_inc_range(I.uses())) {
           auto *UserI = cast<Instruction>(U.getUser());
           U.set(V);
 
+          // Do not bother dealing with unreachable code.
+          if (!DT.isReachableFromEntry(UserI->getParent()))
+            continue;
+
           // If the instruction is used by a PHI node we have already processed
           // we'll need to iterate on the loop body to converge, so add it to
           // the next set.
           if (auto *UserPI = dyn_cast<PHINode>(UserI))
             if (VisitedPHIs.count(UserPI)) {
               Next->insert(UserPI);
               continue;
             }

llvm/test/Transforms/LoopInstSimplify/pr55072.ll

+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
 ; RUN: opt -S %s -passes=loop-instsimplify | FileCheck %s
 ; RUN: opt -S %s -passes='loop-mssa(loop-instsimplify)' -verify-memoryssa | FileCheck %s
 
-; XFAIL: *
-; REQUIRES: asserts
-
 define i32 @test_01() {
-; CHECK-LABEL: test_01
+; CHECK-LABEL: @test_01(
+; CHECK-NEXT:  bb:
+; CHECK-NEXT:    br label [[LOOP:%.*]]
+; CHECK:       loop:
+; CHECK-NEXT:    br label [[LOOP]]
+; CHECK:       unreached:
+; CHECK-NEXT:    ret i32 0
+;
 bb:
   br label %loop
 
 loop:                                              ; preds = %bb, %loop
   %tmp = lshr exact i32 undef, 16
   br label %loop
 
 unreached:                                              ; No predecessors!
   ret i32 %tmp
 }