[CIR][CIRGen] Create a new block after break and continue (#611)

Without this patch, CIR CodeGen continue to generate in the same block after `cir.break` and `cir.continue`, which would cause verification error because `cir.break` and `cir.continue` should appear at the end of blocks. This patch creates a new dangling block after generating `cir.break` and `cir.continue` to fix the issue. This will fix #323.
llvm · Jun 6, 2024 · 59d969c · 59d969c
1 parent 7501afa
commit 59d969c
Show file tree

Hide file tree

Showing 6 changed files with 123 additions and 31 deletions.
diff --git a/clang/lib/CIR/CodeGen/CIRGenStmt.cpp b/clang/lib/CIR/CodeGen/CIRGenStmt.cpp
@@ -551,20 +551,11 @@ mlir::LogicalResult CIRGenFunction::buildGotoStmt(const GotoStmt &S) {
   // info support just yet, look at this again once we have it.
   assert(builder.getInsertionBlock() && "not yet implemented");
 
-  mlir::Block *currBlock = builder.getBlock();
-  mlir::Block *gotoBlock = currBlock;
-  if (!currBlock->empty() &&
-      currBlock->back().hasTrait<mlir::OpTrait::IsTerminator>()) {
-    gotoBlock = builder.createBlock(builder.getBlock()->getParent());
-    builder.setInsertionPointToEnd(gotoBlock);
-  }
-
-  // A goto marks the end of a block, create a new one for codegen after
-  // buildGotoStmt can resume building in that block.
-
   builder.create<mlir::cir::GotoOp>(getLoc(S.getSourceRange()),
                                     S.getLabel()->getName());
 
+  // A goto marks the end of a block, create a new one for codegen after
+  // buildGotoStmt can resume building in that block.
   // Insert the new block to continue codegen after goto.
   builder.createBlock(builder.getBlock()->getParent());
 
@@ -597,11 +588,19 @@ mlir::LogicalResult CIRGenFunction::buildLabel(const LabelDecl *D) {
 mlir::LogicalResult
 CIRGenFunction::buildContinueStmt(const clang::ContinueStmt &S) {
   builder.createContinue(getLoc(S.getContinueLoc()));
+
+  // Insert the new block to continue codegen after the continue statement.
+  builder.createBlock(builder.getBlock()->getParent());
+
   return mlir::success();
 }
 
 mlir::LogicalResult CIRGenFunction::buildBreakStmt(const clang::BreakStmt &S) {
   builder.createBreak(getLoc(S.getBreakLoc()));
+
+  // Insert the new block to continue codegen after the break statement.
+  builder.createBlock(builder.getBlock()->getParent());
+
   return mlir::success();
 }
 

diff --git a/clang/test/CIR/CodeGen/goto.cpp b/clang/test/CIR/CodeGen/goto.cpp
@@ -101,26 +101,6 @@ int shouldGenBranch(int x) {
 // NOFLAT:  ^bb1:  
 // NOFLAT:    cir.label "err"
 
-int shouldCreateBlkForGoto(int a) {
-  switch (a) {
-    case(42):
-      break;
-      goto exit;
-    default:
-      return 0;
-  };
-
-exit:
-  return -1;
-
-}
-// NOFLAT: cir.func @_Z22shouldCreateBlkForGotoi
-// NOFLAT:   case (equal, 42) {
-// NOFLAT:     cir.break
-// NOFLAT:   ^bb1:  // no predecessors
-// NOFLAT:     cir.goto "exit"
-// NOFLAT:   }
-
 void severalLabelsInARow(int a) {
   int b = a;
   goto end1;

diff --git a/clang/test/CIR/CodeGen/loop.cpp b/clang/test/CIR/CodeGen/loop.cpp
@@ -205,3 +205,55 @@ void l6() {
 // CHECK-NEXT:   }
 // CHECK-NEXT:   cir.return
 // CHECK-NEXT: }
+
+void unreachable_after_break() {
+  for (;;) {
+    break;
+    int x = 1;
+  }
+}
+
+// CHECK-NEXT: cir.func @_Z23unreachable_after_breakv()
+// CHECK-NEXT:   cir.scope {
+// CHECK-NEXT:     %0 = cir.alloca !s32i, !cir.ptr<!s32i>, ["x", init] {alignment = 4 : i64}
+// CHECK-NEXT:     cir.for : cond {
+// CHECK-NEXT:       %1 = cir.const #true
+// CHECK-NEXT:       cir.condition(%1)
+// CHECK-NEXT:     } body {
+// CHECK-NEXT:       cir.break
+// CHECK-NEXT:     ^bb1:  // no predecessors
+// CHECK-NEXT:       %1 = cir.const #cir.int<1> : !s32i
+// CHECK-NEXT:       cir.store %1, %0 : !s32i, !cir.ptr<!s32i>
+// CHECK-NEXT:       cir.yield
+// CHECK-NEXT:     } step {
+// CHECK-NEXT:       cir.yield
+// CHECK-NEXT:     }
+// CHECK-NEXT:   }
+// CHECK-NEXT:   cir.return
+// CHECK-NEXT: }
+
+void unreachable_after_continue() {
+  for (;;) {
+    continue;
+    int x = 1;
+  }
+}
+
+// CHECK-NEXT: cir.func @_Z26unreachable_after_continuev()
+// CHECK-NEXT:   cir.scope {
+// CHECK-NEXT:     %0 = cir.alloca !s32i, !cir.ptr<!s32i>, ["x", init] {alignment = 4 : i64}
+// CHECK-NEXT:     cir.for : cond {
+// CHECK-NEXT:       %1 = cir.const #true
+// CHECK-NEXT:       cir.condition(%1)
+// CHECK-NEXT:     } body {
+// CHECK-NEXT:       cir.continue
+// CHECK-NEXT:     ^bb1:  // no predecessors
+// CHECK-NEXT:       %1 = cir.const #cir.int<1> : !s32i
+// CHECK-NEXT:       cir.store %1, %0 : !s32i, !cir.ptr<!s32i>
+// CHECK-NEXT:       cir.yield
+// CHECK-NEXT:     } step {
+// CHECK-NEXT:       cir.yield
+// CHECK-NEXT:     }
+// CHECK-NEXT:   }
+// CHECK-NEXT:   cir.return
+// CHECK-NEXT: }
diff --git a/clang/test/CIR/CodeGen/return.cpp b/clang/test/CIR/CodeGen/return.cpp
@@ -12,3 +12,22 @@ int &ret0(int &x) {
 // CHECK:   cir.store %2, %1 : !cir.ptr<!s32i>, !cir.ptr<!cir.ptr<!s32i>>
 // CHECK:   %3 = cir.load %1 : !cir.ptr<!cir.ptr<!s32i>>, !cir.ptr<!s32i>
 // CHECK:   cir.return %3 : !cir.ptr<!s32i>
+
+int unreachable_after_return() {
+  return 0;
+  return 1;
+}
+
+// CHECK: cir.func @_Z24unreachable_after_returnv
+// CHECK-NEXT:   %0 = cir.alloca !s32i, !cir.ptr<!s32i>, ["__retval"] {alignment = 4 : i64}
+// CHECK-NEXT:   %1 = cir.const #cir.int<0> : !s32i
+// CHECK-NEXT:   cir.store %1, %0 : !s32i, !cir.ptr<!s32i>
+// CHECK-NEXT:   cir.br ^bb1
+// CHECK-NEXT: ^bb1:  // 2 preds: ^bb0, ^bb2
+// CHECK-NEXT:   %2 = cir.load %0 : !cir.ptr<!s32i>, !s32i
+// CHECK-NEXT:   cir.return %2 : !s32i
+// CHECK-NEXT: ^bb2:  // no predecessors
+// CHECK-NEXT:   %3 = cir.const #cir.int<1> : !s32i
+// CHECK-NEXT:   cir.store %3, %0 : !s32i, !cir.ptr<!s32i>
+// CHECK-NEXT:   cir.br ^bb1
+// CHECK-NEXT: }
diff --git a/clang/test/CIR/CodeGen/switch-unreachable-after-break.cpp b/clang/test/CIR/CodeGen/switch-unreachable-after-break.cpp
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -std=c++17 -triple x86_64-unknown-linux-gnu -fclangir -emit-cir %s -o %t.cir
+// RUN: FileCheck --input-file=%t.cir %s
+// XFAIL: *
+
+void unreachable_after_break(int a) {
+  switch(a) {
+  case 0:
+    break;
+    break;
+    int x = 1;
+  }
+}
+
+int unreachable_after_return(int a) {
+  switch (a) {
+  case 0:
+    return 0;
+    return 1;
+    int x = 1;
+  }
+  return 2;
+}
diff --git a/clang/test/CIR/CodeGen/switch.cpp b/clang/test/CIR/CodeGen/switch.cpp
@@ -327,3 +327,23 @@ void fallthrough(int x) {
 // CHECK-NEXT:      }
 // CHECK-NEXT:      ]
 // CHECK-NEXT:    }
+
+int unreachable_after_break_1(int a) {
+  switch (a) {
+    case(42):
+      break;
+      goto exit;
+    default:
+      return 0;
+  };
+
+exit:
+  return -1;
+
+}
+// CHECK: cir.func @_Z25unreachable_after_break_1i
+// CHECK:   case (equal, 42) {
+// CHECK:     cir.break
+// CHECK:   ^bb1:  // no predecessors
+// CHECK:     cir.goto "exit"
+// CHECK:   }