[llvm] r330564 - [PM/LoopUnswitch] Remove a buggy assert in the new loop unswitch.

[Chandler Carruth via llvm-commits]

Author: chandlerc
Date: Sun Apr 22 23:58:36 2018
New Revision: 330564

URL: http://llvm.org/viewvc/llvm-project?rev=330564&view=rev
Log:
[PM/LoopUnswitch] Remove a buggy assert in the new loop unswitch.

The condition this was asserting doesn't actually hold. I've added
comments to explain why, removed the assert, and added a fun test case
reduced out of 403.gcc.

Modified:
    llvm/trunk/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp
    llvm/trunk/test/Transforms/SimpleLoopUnswitch/nontrivial-unswitch.ll

Modified: llvm/trunk/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp?rev=330564&r1=330563&r2=330564&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp Sun Apr 22 23:58:36 2018
@@ -1364,9 +1364,10 @@ static SmallPtrSet<const BasicBlock *, 1
           continue;
 
         // Insert all of the blocks (other than those already present) into
-        // the loop set. The only block we expect to already be in the set is
-        // the one we used to find this loop as we immediately handle the
-        // others the first time we encounter the loop.
+        // the loop set. We expect at least the block that led us to find the
+        // inner loop to be in the block set, but we may also have other loop
+        // blocks if they were already enqueued as predecessors of some other
+        // outer loop block.
         for (auto *InnerBB : InnerL->blocks()) {
           if (InnerBB == BB) {
             assert(LoopBlockSet.count(InnerBB) &&
@@ -1374,9 +1375,7 @@ static SmallPtrSet<const BasicBlock *, 1
             continue;
           }
 
-          bool Inserted = LoopBlockSet.insert(InnerBB).second;
-          (void)Inserted;
-          assert(Inserted && "Should only insert an inner loop once!");
+          LoopBlockSet.insert(InnerBB);
         }
 
         // Add the preheader to the worklist so we will continue past the

Modified: llvm/trunk/test/Transforms/SimpleLoopUnswitch/nontrivial-unswitch.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/SimpleLoopUnswitch/nontrivial-unswitch.ll?rev=330564&r1=330563&r2=330564&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/SimpleLoopUnswitch/nontrivial-unswitch.ll (original)
+++ llvm/trunk/test/Transforms/SimpleLoopUnswitch/nontrivial-unswitch.ll Sun Apr 22 23:58:36 2018
@@ -2380,3 +2380,58 @@ loop_latch:
 loop_exit:
   ret void
 }
+
+; A test reduced out of 403.gcc with interesting nested loops that trigger
+; multiple unswitches. A key component of this test is that there are multiple
+; paths to reach an inner loop after unswitching, and one of them is via the
+; predecessors of the unswitched loop header. That can allow us to find the loop
+; through multiple different paths.
+define void @test21(i1 %a, i1 %b) {
+; CHECK-LABEL: @test21(
+bb:
+  br label %bb3
+; CHECK-NOT:     br i1 %a
+;
+; CHECK:         br i1 %a, label %[[BB_SPLIT_US:.*]], label %[[BB_SPLIT:.*]]
+;
+; CHECK-NOT:     br i1 %a
+; CHECK-NOT:     br i1 %b
+;
+; CHECK:       [[BB_SPLIT]]:
+; CHECK:         br i1 %b
+;
+; CHECK-NOT:     br i1 %a
+; CHECK-NOT:     br i1 %b
+
+bb3:
+  %tmp1.0 = phi i32 [ 0, %bb ], [ %tmp1.3, %bb23 ]
+  br label %bb7
+
+bb7:
+  %tmp.0 = phi i1 [ true, %bb3 ], [ false, %bb19 ]
+  %tmp1.1 = phi i32 [ %tmp1.0, %bb3 ], [ %tmp1.2.lcssa, %bb19 ]
+  br i1 %tmp.0, label %bb11.preheader, label %bb23
+
+bb11.preheader:
+  br i1 %a, label %bb19, label %bb14.lr.ph
+
+bb14.lr.ph:
+  br label %bb14
+
+bb14:
+  %tmp2.02 = phi i32 [ 0, %bb14.lr.ph ], [ 1, %bb14 ]
+  br i1 %b, label %bb11.bb19_crit_edge, label %bb14
+
+bb11.bb19_crit_edge:
+  %split = phi i32 [ %tmp2.02, %bb14 ]
+  br label %bb19
+
+bb19:
+  %tmp1.2.lcssa = phi i32 [ %split, %bb11.bb19_crit_edge ], [ %tmp1.1, %bb11.preheader ]
+  %tmp21 = icmp eq i32 %tmp1.2.lcssa, 0
+  br i1 %tmp21, label %bb23, label %bb7
+
+bb23:
+  %tmp1.3 = phi i32 [ %tmp1.2.lcssa, %bb19 ], [ %tmp1.1, %bb7 ]
+  br label %bb3
+}