[StructurizeCFG] Remove imposible case, associated test, and replace by assert

[ruiling]

I think it should be ok to remove the code, but I would rather we keep the tests, the test is hitting the limitation of StructurizeCFG. The test contains infinite loop in its CFG. Maybe we can update the comment in the test to say this, and see whether we could solve this problem later.

[jmmartinez]

In D134439#3810796, @ruiling wrote:

I think it should be ok to remove the code, but I would rather we keep the tests, the test is hitting the limitation of StructurizeCFG. The test contains infinite loop in its CFG. Maybe we can update the comment in the test to say this, and see whether we could solve this problem later.

Whay do you think about removing this test and introducing a separate infinite-loop.ll test ? I'd prefer a test that matches what we expect StructurizeCFG to do on an infinite loop (in this particular case, nothing) than the current test.

[ruiling]

In D134439#3818081, @jmmartinez wrote:

In D134439#3810796, @ruiling wrote:

I think it should be ok to remove the code, but I would rather we keep the tests, the test is hitting the limitation of StructurizeCFG. The test contains infinite loop in its CFG. Maybe we can update the comment in the test to say this, and see whether we could solve this problem later.

Whay do you think about removing this test and introducing a separate infinite-loop.ll test ? I'd prefer a test that matches what we expect StructurizeCFG to do on an infinite loop (in this particular case, nothing) than the current test.

sounds good to me.

[jmmartinez]

• Test case for infinite loops added

[ruiling]

llvm/test/Transforms/StructurizeCFG/infinite-loop.ll

8

I think you can just remove this one, we don't need to test against 'br undef'. There has been discussion to remove 'br undef' in test, see https://discourse.llvm.org/t/please-dont-use-br-undef-in-tests-aka-please-avoid-test-cases-with-ub/63115. Other parts LGTM.

[jmmartinez]

Removed undef branch test case

[ruiling]

Thanks!

llvm/lib/Transforms/Scalar/StructurizeCFG.cpp

if (!isPredictableTrue(Node))
     LoopStart = needPrefix(true);
 
   LoopEnd = Loops[Node->getEntry()];
   wireFlow(false, LoopEnd);
   while (!Visited.count(LoopEnd)) {
     handleLoops(false, LoopEnd);
   }
 
-  // If the start of the loop is the entry block, we can't branch to it so
-  // insert a new dummy entry block.
-  Function *LoopFunc = LoopStart->getParent();
-  if (LoopStart == &LoopFunc->getEntryBlock()) {
-    LoopStart->setName("entry.orig");
-
-    BasicBlock *NewEntry =
-      BasicBlock::Create(LoopStart->getContext(),
-                         "entry",
-                         LoopFunc,
-                         LoopStart);
-    BranchInst::Create(LoopStart, NewEntry);
-    DT->setNewRoot(NewEntry);
-  }
+  assert(LoopStart != &LoopStart->getParent()->getEntryBlock());
 
   // Create an extra loop end node
   LoopEnd = needPrefix(false);
   BasicBlock *Next = needPostfix(LoopEnd, ExitUseAllowed);
   LoopConds.push_back(BranchInst::Create(Next, LoopStart,
                                          BoolUndef, LoopEnd));
   addPhiValues(LoopEnd, LoopStart);
   setPrevNode(Next);

llvm/test/Transforms/StructurizeCFG/infinite-loop.ll

+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt -S -o - -structurizecfg -verify-dom-info < %s | FileCheck %s
+
+; This test hits a limitation in StructurizeCFG: it doesn't handle infinite-loops.
+; In this case, the IR remains unchanged.
+
+define void @infinite_loop_false(i32 addrspace(1)* %out, i1 %cond) {
+; CHECK-LABEL: @infinite_loop_false(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    br i1 [[COND:%.*]], label [[FOR_END:%.*]], label [[FOR_BODY:%.*]]
+; CHECK:       for.body:
+; CHECK-NEXT:    store volatile i32 999, i32 addrspace(1)* [[OUT:%.*]], align 4
+; CHECK-NEXT:    br label [[FOR_BODY]]
+; CHECK:       for.end:
+; CHECK-NEXT:    ret void
+;
+entry:
+  br i1 %cond, label %for.end, label %for.body
+
+for.body:
+  store volatile i32 999, i32 addrspace(1)* %out, align 4
+  br label %for.body
+
+for.end:
+  ret void
+}
+
+define void @infinite_loop_on_branch(i32 addrspace(1)* %out, i1 %cond) {
+; CHECK-LABEL: @infinite_loop_on_branch(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    br i1 [[COND:%.*]], label [[IF_THEN:%.*]], label [[IF_ELSE:%.*]]
+; CHECK:       if.then:
+; CHECK-NEXT:    br label [[FOR_BODY:%.*]]
+; CHECK:       for.body:
+; CHECK-NEXT:    store volatile i32 999, i32 addrspace(1)* [[OUT:%.*]], align 4
+; CHECK-NEXT:    br label [[FOR_BODY]]
+; CHECK:       if.else:
+; CHECK-NEXT:    store volatile i32 111, i32 addrspace(1)* [[OUT]], align 4
+; CHECK-NEXT:    br label [[EXIT:%.*]]
+; CHECK:       exit:
+; CHECK-NEXT:    ret void
+;
+entry:
+  br i1 %cond, label %if.then, label %if.else
+
+if.then:
+  br label %for.body
+
+for.body:
+  store volatile i32 999, i32 addrspace(1)* %out, align 4
+  br label %for.body
+
+if.else:
+  store volatile i32 111, i32 addrspace(1)* %out, align 4
+  br label %exit
+
+exit:
+  ret void
+}

llvm/test/Transforms/StructurizeCFG/no-branch-to-entry.ll

-; XFAIL: *
-
-; This test used to generate a region that caused it to delete the entry block,
-; but it does not anymore after the changes to handling of infinite loops in the
-; PostDominatorTree.
-; TODO: This should be either replaced with another IR or deleted completely.
-
-; RUN: opt -S -o - -structurizecfg -verify-dom-info < %s | FileCheck %s
-
-; CHECK-LABEL: @no_branch_to_entry_undef(
-; CHECK: entry:
-; CHECK-NEXT: br label %entry.orig
-define void @no_branch_to_entry_undef(i32 addrspace(1)* %out) {
-entry:
-  br i1 undef, label %for.end, label %for.body
-
-for.body:                                         ; preds = %entry, %for.body
-  store i32 999, i32 addrspace(1)* %out, align 4
-  br label %for.body
-
-for.end:                                          ; preds = %Flow
-  ret void
-}
-
-; CHECK-LABEL: @no_branch_to_entry_true(
-; CHECK: entry:
-; CHECK-NEXT: br label %entry.orig
-define void @no_branch_to_entry_true(i32 addrspace(1)* %out) {
-entry:
-  br i1 true, label %for.end, label %for.body
-
-for.body:                                         ; preds = %entry, %for.body
-  store i32 999, i32 addrspace(1)* %out, align 4
-  br label %for.body
-
-for.end:                                          ; preds = %Flow
-  ret void
-}