Fix a phase-ordering problem in SimplifyCFG.

[resistor]

Summary

Switch simplification could sometimes fail to notice when an
intermediate case removal caused the switch condition to become
constant. This would cause the switch to be simplified into a
conditional branch rather than a direct branch.

Most of the time this didn't matter, except that occasionally
downstream parts of SimplifyCFG expect tautological branches to
already have been eliminated. The missed handling in switch
simplification would cause an assertion failure in the downstream
code.

Triggering the assertion failure is fairly sensitive to the exact
order of various simplifications.

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

[nikic]

llvm/test/Transforms/SimplifyCFG/switch-simplify-crash2.ll

3

Outside PhaseOrdering, tests should not use the full optimization pipeline. Here is a reduction of your test case:

; RUN: opt -S -passes="simplifycfg<forward-switch-cond;no-keep-loops>" < %s | FileCheck %s

define i8 @test() {
entry:
  br label %loop

loop:
  %phi1 = phi i8 [ 0, %entry ], [ %phi2, %loop2 ]
  br label %loop2

loop2:
  %phi2 = phi i8 [ %phi1, %loop ], [ 0, %loop2 ]
  switch i8 %phi2, label %loop [
    i8 0, label %loop2
    i8 1, label %exit
  ]

exit:
  ret i8 0
}

Please also reference the issue.

[resistor]

Reduce test case.

[nikic]

llvm/lib/Transforms/Utils/Local.cpp

241

I think this may cause problems if we have the following case: We have a case 1 => foo, case 2 => default, then removing default folds CI to 1, but at this point we have already visited case 1, so TheOnlyDest will remain nullptr, and we'll set it to the default destination below, which would be the wrong one in that case.

[resistor]

Restart case iteration when we fold the switch condition.

[nikic]

LG

llvm/lib/Transforms/Utils/Local.cpp

243–244

[resistor]

Incorporate suggestion

llvm/lib/Transforms/Utils/Local.cpp

for (auto i = SI->case_begin(), e = SI->case_end(); i != e;) {
                           MDBuilder(BB->getContext()).
                           createBranchWeights(Weights));
         }
         // Remove this entry.
         BasicBlock *ParentBB = SI->getParent();
         DefaultDest->removePredecessor(ParentBB);
         i = SI->removeCase(i);
         e = SI->case_end();
+
+        // Removing this case may have made the condition constant. In that
+        // case, update CI and restart iteration through the cases.
+        if (auto *NewCI = dyn_cast<ConstantInt>(SI->getCondition())) {
+          CI = NewCI;
+          i = SI->case_begin();
+        }
+
         Changed = true;
         continue;
       }
 
       // Otherwise, check to see if the switch only branches to one destination.
       // We do this by reseting "TheOnlyDest" to null when we find two non-equal
       // destinations.
       if (i->getCaseSuccessor() != TheOnlyDest)

llvm/test/Transforms/SimplifyCFG/switch-simplify-crash2.ll

+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt -S -passes="simplifycfg<forward-switch-cond;no-keep-loops>" < %s | FileCheck %s
+
+define i8 @test() {
+; CHECK-LABEL: @test(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    br label [[LOOP2:%.*]]
+; CHECK:       loop2:
+; CHECK-NEXT:    br label [[LOOP2]]
+;
+entry:
+  br label %loop
+
+loop:
+  %phi1 = phi i8 [ 0, %entry ], [ %phi2, %loop2 ]
+  br label %loop2
+
+loop2:
+  %phi2 = phi i8 [ %phi1, %loop ], [ 0, %loop2 ]
+  switch i8 %phi2, label %loop [
+  i8 0, label %loop2
+  i8 1, label %exit
+  ]
+
+exit:
+  ret i8 0
+}