https://github.com/xlauko created 
https://github.com/llvm/llvm-project/pull/191682

None

>From 7efde37f6b30b55c97801dc219e78c62705d00c4 Mon Sep 17 00:00:00 2001
From: xlauko <[email protected]>
Date: Sun, 12 Apr 2026 06:06:41 +0200
Subject: [PATCH] WIP

---
 clang/include/clang/CIR/Dialect/IR/CIROps.td |  13 +-
 clang/lib/CIR/Dialect/IR/CIRDialect.cpp      | 124 +------------------
 clang/test/CIR/IR/if.cir                     |  57 +++++++++
 mlir/include/mlir/IR/OpBase.td               |  15 ++-
 mlir/include/mlir/IR/OpDefinition.h          |  94 +++++++++-----
 mlir/tools/mlir-tblgen/OpFormatGen.cpp       |   5 +-
 6 files changed, 148 insertions(+), 160 deletions(-)
 create mode 100644 clang/test/CIR/IR/if.cir

diff --git a/clang/include/clang/CIR/Dialect/IR/CIROps.td 
b/clang/include/clang/CIR/Dialect/IR/CIROps.td
index f72d891ecd941..b1d296765e9bc 100644
--- a/clang/include/clang/CIR/Dialect/IR/CIROps.td
+++ b/clang/include/clang/CIR/Dialect/IR/CIROps.td
@@ -836,7 +836,8 @@ def CIR_ReturnOp : CIR_Op<"return", [
 
 def CIR_IfOp : CIR_Op<"if", [
   DeclareOpInterfaceMethods<RegionBranchOpInterface, ["getSuccessorInputs"]>,
-  RecursivelySpeculatable, AutomaticAllocationScope, NoRegionArguments
+  RecursivelySpeculatable, AutomaticAllocationScope, NoRegionArguments,
+  ImplicitDefaultTerminator<"cir::YieldOp">
 ]> {
   let summary = "the if-then-else operation";
   let description = [{
@@ -872,7 +873,9 @@ def CIR_IfOp : CIR_Op<"if", [
   }];
   let arguments = (ins CIR_BoolType:$condition);
   let regions = (region AnyRegion:$thenRegion, AnyRegion:$elseRegion);
-  let hasCustomAssemblyFormat=1;
+  let assemblyFormat = [{
+    $condition attr-dict-with-keyword $thenRegion (`else` $elseRegion^)?
+  }];
   let skipDefaultBuilders=1;
   let builders = [
     OpBuilder<(ins "mlir::Value":$cond, "bool":$withElseRegion,
@@ -1122,7 +1125,7 @@ def CIR_ResumeFlatOp : CIR_Op<"resume.flat", [
 def CIR_ScopeOp : CIR_Op<"scope", [
   DeclareOpInterfaceMethods<RegionBranchOpInterface, ["getSuccessorInputs"]>,
   RecursivelySpeculatable, AutomaticAllocationScope, NoRegionArguments,
-  RecursiveMemoryEffects
+  RecursiveMemoryEffects, ImplicitDefaultTerminator<"cir::YieldOp">
 ]> {
   let summary = "Represents a C/C++ scope";
   let description = [{
@@ -1153,7 +1156,7 @@ def CIR_ScopeOp : CIR_Op<"scope", [
   let hasVerifier = 1;
   let skipDefaultBuilders = 1;
   let assemblyFormat = [{
-    custom<OmittedTerminatorRegion>($scopeRegion) (`:` type($results)^)? 
attr-dict
+    $scopeRegion (`:` type($results)^)? attr-dict
   }];
 
   let extraClassDeclaration = [{
@@ -2804,7 +2807,7 @@ def CIR_TLSModel : CIR_I32EnumAttr<"TLS_Model", "TLS 
model", [
 def CIR_GlobalOp : CIR_Op<"global", [
   DeclareOpInterfaceMethods<RegionBranchOpInterface, ["getSuccessorInputs"]>,
   DeclareOpInterfaceMethods<CIRGlobalValueInterface>,
-  NoRegionArguments
+  NoRegionArguments, ImplicitDefaultTerminator<"cir::YieldOp">
 ]> {
   let summary = "Declare or define a global variable";
   let description = [{
diff --git a/clang/lib/CIR/Dialect/IR/CIRDialect.cpp 
b/clang/lib/CIR/Dialect/IR/CIRDialect.cpp
index 8ccc83a25537b..28acb0842afeb 100644
--- a/clang/lib/CIR/Dialect/IR/CIRDialect.cpp
+++ b/clang/lib/CIR/Dialect/IR/CIRDialect.cpp
@@ -169,44 +169,6 @@ static ParseResult parseCIRKeyword(AsmParser &parser, 
RetTy &result) {
   return success();
 }
 
-// Check if a region's termination omission is valid and, if so, creates and
-// inserts the omitted terminator into the region.
-static LogicalResult ensureRegionTerm(OpAsmParser &parser, Region &region,
-                                      SMLoc errLoc) {
-  Location eLoc = parser.getEncodedSourceLoc(parser.getCurrentLocation());
-  OpBuilder builder(parser.getBuilder().getContext());
-
-  // Insert empty block in case the region is empty to ensure the terminator
-  // will be inserted
-  if (region.empty())
-    builder.createBlock(&region);
-
-  Block &block = region.back();
-  // Region is properly terminated: nothing to do.
-  if (!block.empty() && block.back().hasTrait<OpTrait::IsTerminator>())
-    return success();
-
-  // Check for invalid terminator omissions.
-  if (!region.hasOneBlock())
-    return parser.emitError(errLoc,
-                            "multi-block region must not omit terminator");
-
-  // Terminator was omitted correctly: recreate it.
-  builder.setInsertionPointToEnd(&block);
-  cir::YieldOp::create(builder, eLoc);
-  return success();
-}
-
-// True if the region's terminator should be omitted.
-static bool omitRegionTerm(mlir::Region &r) {
-  const auto singleNonEmptyBlock = r.hasOneBlock() && !r.back().empty();
-  const auto yieldsNothing = [&r]() {
-    auto y = dyn_cast<cir::YieldOp>(r.back().getTerminator());
-    return y && y.getArgs().empty();
-  };
-  return singleNonEmptyBlock && yieldsNothing();
-}
-
 
//===----------------------------------------------------------------------===//
 // InlineKindAttr (FIXME: remove once FuncOp uses assembly format)
 
//===----------------------------------------------------------------------===//
@@ -246,24 +208,6 @@ void printInlineKindAttr(OpAsmPrinter &p, 
cir::InlineKindAttr inlineKindAttr) {
 // CIR Custom Parsers/Printers
 
//===----------------------------------------------------------------------===//
 
-static mlir::ParseResult parseOmittedTerminatorRegion(mlir::OpAsmParser 
&parser,
-                                                      mlir::Region &region) {
-  auto regionLoc = parser.getCurrentLocation();
-  if (parser.parseRegion(region))
-    return failure();
-  if (ensureRegionTerm(parser, region, regionLoc).failed())
-    return failure();
-  return success();
-}
-
-static void printOmittedTerminatorRegion(mlir::OpAsmPrinter &printer,
-                                         cir::ScopeOp &op,
-                                         mlir::Region &region) {
-  printer.printRegion(region,
-                      /*printEntryBlockArgs=*/false,
-                      /*printBlockTerminators=*/!omitRegionTerm(region));
-}
-
 mlir::OptionalParseResult
 parseGlobalAddressSpaceValue(mlir::AsmParser &p,
                              mlir::ptr::MemorySpaceAttrInterface &attr);
@@ -1156,62 +1100,6 @@ mlir::LogicalResult cir::ReturnOp::verify() {
 // IfOp
 
//===----------------------------------------------------------------------===//
 
-ParseResult cir::IfOp::parse(OpAsmParser &parser, OperationState &result) {
-  // create the regions for 'then'.
-  result.regions.reserve(2);
-  Region *thenRegion = result.addRegion();
-  Region *elseRegion = result.addRegion();
-
-  mlir::Builder &builder = parser.getBuilder();
-  OpAsmParser::UnresolvedOperand cond;
-  Type boolType = cir::BoolType::get(builder.getContext());
-
-  if (parser.parseOperand(cond) ||
-      parser.resolveOperand(cond, boolType, result.operands))
-    return failure();
-
-  // Parse 'then' region.
-  mlir::SMLoc parseThenLoc = parser.getCurrentLocation();
-  if (parser.parseRegion(*thenRegion, /*arguments=*/{}, /*argTypes=*/{}))
-    return failure();
-
-  if (ensureRegionTerm(parser, *thenRegion, parseThenLoc).failed())
-    return failure();
-
-  // If we find an 'else' keyword, parse the 'else' region.
-  if (!parser.parseOptionalKeyword("else")) {
-    mlir::SMLoc parseElseLoc = parser.getCurrentLocation();
-    if (parser.parseRegion(*elseRegion, /*arguments=*/{}, /*argTypes=*/{}))
-      return failure();
-    if (ensureRegionTerm(parser, *elseRegion, parseElseLoc).failed())
-      return failure();
-  }
-
-  // Parse the optional attribute list.
-  if (parser.parseOptionalAttrDict(result.attributes))
-    return failure();
-  return success();
-}
-
-void cir::IfOp::print(OpAsmPrinter &p) {
-  p << " " << getCondition() << " ";
-  mlir::Region &thenRegion = this->getThenRegion();
-  p.printRegion(thenRegion,
-                /*printEntryBlockArgs=*/false,
-                /*printBlockTerminators=*/!omitRegionTerm(thenRegion));
-
-  // Print the 'else' regions if it exists and has a block.
-  mlir::Region &elseRegion = this->getElseRegion();
-  if (!elseRegion.empty()) {
-    p << " else ";
-    p.printRegion(elseRegion,
-                  /*printEntryBlockArgs=*/false,
-                  /*printBlockTerminators=*/!omitRegionTerm(elseRegion));
-  }
-
-  p.printOptionalAttrDict(getOperation()->getAttrs());
-}
-
 /// Default callback for IfOp builders.
 void cir::buildTerminatedBody(OpBuilder &builder, Location loc) {
   // add cir.yield to end of the block
@@ -1853,11 +1741,11 @@ static ParseResult 
parseGlobalOpTypeAndInitialValue(OpAsmParser &parser,
     if (!parser.parseOptionalKeyword("ctor")) {
       if (parser.parseColonType(opTy))
         return failure();
-      auto parseLoc = parser.getCurrentLocation();
       if (parser.parseRegion(ctorRegion, /*arguments=*/{}, /*argTypes=*/{}))
         return failure();
-      if (ensureRegionTerm(parser, ctorRegion, parseLoc).failed())
-        return failure();
+      cir::GlobalOp::ensureTerminator(
+          ctorRegion, parser.getBuilder(),
+          parser.getEncodedSourceLoc(parser.getCurrentLocation()));
     } else {
       // Parse constant with initializer, examples:
       //  cir.global @y = 3.400000e+00 : f32
@@ -1874,11 +1762,11 @@ static ParseResult 
parseGlobalOpTypeAndInitialValue(OpAsmParser &parser,
     // Parse destructor, example:
     //   dtor { ... }
     if (!parser.parseOptionalKeyword("dtor")) {
-      auto parseLoc = parser.getCurrentLocation();
       if (parser.parseRegion(dtorRegion, /*arguments=*/{}, /*argTypes=*/{}))
         return failure();
-      if (ensureRegionTerm(parser, dtorRegion, parseLoc).failed())
-        return failure();
+      cir::GlobalOp::ensureTerminator(
+          dtorRegion, parser.getBuilder(),
+          parser.getEncodedSourceLoc(parser.getCurrentLocation()));
     }
   }
 
diff --git a/clang/test/CIR/IR/if.cir b/clang/test/CIR/IR/if.cir
new file mode 100644
index 0000000000000..fa161c5c21a3d
--- /dev/null
+++ b/clang/test/CIR/IR/if.cir
@@ -0,0 +1,57 @@
+// RUN: cir-opt %s --verify-roundtrip | FileCheck %s
+
+!s32i = !cir.int<s, 32>
+
+module {
+
+// Omitted yield is auto-inserted and re-omitted on print.
+cir.func @if_omitted_yield(%arg0 : !cir.bool) {
+  cir.if %arg0 {
+  }
+  cir.return
+}
+// CHECK-LABEL: cir.func @if_omitted_yield
+// CHECK:         cir.if %{{.*}} {
+// CHECK-NEXT:    }
+
+// Non-yield terminator is preserved on roundtrip.
+cir.func @if_non_yield_terminator(%arg0 : !cir.bool) -> !s32i {
+  cir.if %arg0 {
+    %0 = cir.const #cir.int<42> : !s32i
+    cir.return %0 : !s32i
+  }
+  %1 = cir.const #cir.int<0> : !s32i
+  cir.return %1 : !s32i
+}
+// CHECK-LABEL: cir.func @if_non_yield_terminator
+// CHECK:         cir.if %{{.*}} {
+// CHECK:           cir.return %{{.*}} : !s32i
+// CHECK-NEXT:    }
+
+// Optional else region omitted.
+cir.func @if_no_else(%arg0 : !cir.bool) {
+  cir.if %arg0 {
+    cir.yield
+  }
+  cir.return
+}
+// CHECK-LABEL: cir.func @if_no_else
+// CHECK:         cir.if %{{.*}} {
+// CHECK-NEXT:    }
+// CHECK-NOT:     else
+
+// Both then and else present.
+cir.func @if_with_else(%arg0 : !cir.bool) {
+  cir.if %arg0 {
+    cir.yield
+  } else {
+    cir.yield
+  }
+  cir.return
+}
+// CHECK-LABEL: cir.func @if_with_else
+// CHECK:         cir.if %{{.*}} {
+// CHECK-NEXT:    } else {
+// CHECK-NEXT:    }
+
+}
diff --git a/mlir/include/mlir/IR/OpBase.td b/mlir/include/mlir/IR/OpBase.td
index 7f36e6c74c7f7..5d371fc31666c 100644
--- a/mlir/include/mlir/IR/OpBase.td
+++ b/mlir/include/mlir/IR/OpBase.td
@@ -122,9 +122,18 @@ def ElementwiseMappable : TraitList<[
 // Op's regions have a single block.
 def SingleBlock : NativeOpTrait<"SingleBlock">, StructuralOpTrait;
 
-class SingleBlockImplicitTerminatorImpl<string op>
-    : ParamNativeOpTrait<"SingleBlockImplicitTerminator", op, [SingleBlock]>,
-      StructuralOpTrait;
+/// Base inner trait providing ODS implicit-terminator printer/parser 
machinery.
+class ImplicitTerminator<string op>
+    : ParamNativeOpTrait<"ImplicitDefaultTerminator", op>, StructuralOpTrait;
+
+/// Any terminator accepted; T auto-inserted if absent. No SingleBlock.
+class ImplicitDefaultTerminator<string op>
+    : TraitList<[ImplicitTerminator<op>]>;
+
+class SingleBlockImplicitTerminatorImpl<string op> : ImplicitTerminator<op> {
+  let trait = "SingleBlockImplicitTerminator<"#op#">::Impl";
+  let dependentTraits = [SingleBlock];
+}
 
 // Op's regions have a single block with the specified terminator.
 class SingleBlockImplicitTerminator<string op>
diff --git a/mlir/include/mlir/IR/OpDefinition.h 
b/mlir/include/mlir/IR/OpDefinition.h
index e886ac45675e2..9d9acb4bda0b5 100644
--- a/mlir/include/mlir/IR/OpDefinition.h
+++ b/mlir/include/mlir/IR/OpDefinition.h
@@ -955,29 +955,75 @@ struct SingleBlock : public TraitBase<ConcreteType, 
SingleBlock> {
 };
 
 
//===----------------------------------------------------------------------===//
-// SingleBlockImplicitTerminator
+// ImplicitDefaultTerminator / SingleBlockImplicitTerminator
 
//===----------------------------------------------------------------------===//
 
-/// This class provides APIs and verifiers for ops with regions having a single
-/// block that must terminate with `TerminatorOpType`.
+namespace detail {
+/// Shared base for implicit-terminator traits. Provides `ensureTerminator`,
+/// `ImplicitTerminatorOpT`, and the terminator builder. Not a trait itself —
+/// mixed into trait Impl classes via multiple inheritance.
 template <typename TerminatorOpType>
-struct SingleBlockImplicitTerminator {
+struct ImplicitTerminatorBase {
+  using ImplicitTerminatorOpT = TerminatorOpType;
+
+  /// Ensure that the given region has the terminator required by this trait.
+  /// If OpBuilder is provided, use it to build the terminator and notify the
+  /// OpBuilder listeners accordingly. If only a Builder is provided, locally
+  /// construct an OpBuilder with no listeners; this should only be used if no
+  /// OpBuilder is available at the call site, e.g., in the parser.
+  static void ensureTerminator(Region &region, Builder &builder, Location loc) 
{
+    ::mlir::impl::ensureRegionTerminator(region, builder, loc, 
buildTerminator);
+  }
+  static void ensureTerminator(Region &region, OpBuilder &builder,
+                               Location loc) {
+    ::mlir::impl::ensureRegionTerminator(region, builder, loc, 
buildTerminator);
+  }
+
+private:
+  static Operation *buildTerminator(OpBuilder &builder, Location loc) {
+    OperationState state(loc, TerminatorOpType::getOperationName());
+    TerminatorOpType::build(builder, state);
+    return Operation::create(state);
+  }
+};
+} // namespace detail
+
+/// Ops whose regions may end with any IsTerminator op. `TerminatorOpType` is
+/// inserted as the default when a region block has no terminator. Does not
+/// enforce SingleBlock — regions may have multiple blocks.
+template <typename TerminatorOpType>
+struct ImplicitDefaultTerminator {
   template <typename ConcreteType>
-  class Impl : public TraitBase<ConcreteType, SingleBlockImplicitTerminator<
-                                                  TerminatorOpType>::Impl> {
-  private:
-    /// Builds a terminator operation without relying on OpBuilder APIs to 
avoid
-    /// cyclic header inclusion.
-    static Operation *buildTerminator(OpBuilder &builder, Location loc) {
-      OperationState state(loc, TerminatorOpType::getOperationName());
-      TerminatorOpType::build(builder, state);
-      return Operation::create(state);
+  class Impl
+      : public TraitBase<ConcreteType,
+                         ImplicitDefaultTerminator<TerminatorOpType>::Impl>,
+        public detail::ImplicitTerminatorBase<TerminatorOpType> {
+  public:
+    static LogicalResult verifyRegionTrait(Operation *op) {
+      for (unsigned i = 0, e = op->getNumRegions(); i < e; ++i) {
+        Region &region = op->getRegion(i);
+        if (region.empty())
+          continue;
+        if (region.front().back().hasTrait<OpTrait::IsTerminator>())
+          continue;
+        return op->emitOpError("expects region #")
+               << i << " to end with a terminator";
+      }
+      return success();
     }
+  };
+};
 
+/// Ops with regions having a single block that must terminate with
+/// `TerminatorOpType`.
+template <typename TerminatorOpType>
+struct SingleBlockImplicitTerminator {
+  template <typename ConcreteType>
+  class Impl
+      : public TraitBase<ConcreteType,
+                         
SingleBlockImplicitTerminator<TerminatorOpType>::Impl>,
+        public detail::ImplicitTerminatorBase<TerminatorOpType> {
   public:
-    /// The type of the operation used as the implicit terminator type.
-    using ImplicitTerminatorOpT = TerminatorOpType;
-
     static LogicalResult verifyRegionTrait(Operation *op) {
       for (unsigned i = 0, e = op->getNumRegions(); i < e; ++i) {
         Region &region = op->getRegion(i);
@@ -1000,22 +1046,6 @@ struct SingleBlockImplicitTerminator {
 
       return success();
     }
-
-    /// Ensure that the given region has the terminator required by this trait.
-    /// If OpBuilder is provided, use it to build the terminator and notify the
-    /// OpBuilder listeners accordingly. If only a Builder is provided, locally
-    /// construct an OpBuilder with no listeners; this should only be used if 
no
-    /// OpBuilder is available at the call site, e.g., in the parser.
-    static void ensureTerminator(Region &region, Builder &builder,
-                                 Location loc) {
-      ::mlir::impl::ensureRegionTerminator(region, builder, loc,
-                                           buildTerminator);
-    }
-    static void ensureTerminator(Region &region, OpBuilder &builder,
-                                 Location loc) {
-      ::mlir::impl::ensureRegionTerminator(region, builder, loc,
-                                           buildTerminator);
-    }
   };
 };
 
diff --git a/mlir/tools/mlir-tblgen/OpFormatGen.cpp 
b/mlir/tools/mlir-tblgen/OpFormatGen.cpp
index ff51fb403ffc8..e1f61bc5cd23c 100644
--- a/mlir/tools/mlir-tblgen/OpFormatGen.cpp
+++ b/mlir/tools/mlir-tblgen/OpFormatGen.cpp
@@ -350,7 +350,7 @@ struct OperationFormat {
     resultTypes.resize(op.getNumResults(), TypeResolution());
 
     hasImplicitTermTrait = llvm::any_of(op.getTraits(), [](const Trait &trait) 
{
-      return trait.getDef().isSubClassOf("SingleBlockImplicitTerminatorImpl");
+      return trait.getDef().isSubClassOf("ImplicitTerminator");
     });
 
     hasSingleBlockTrait = op.getTrait("::mlir::OpTrait::SingleBlock");
@@ -1958,7 +1958,8 @@ static const char 
*regionSingleBlockImplicitTerminatorPrinterCode = R"(
   {
     bool printTerminator = true;
     if (auto *term = {0}.empty() ? nullptr : {0}.begin()->getTerminator()) {{
-      printTerminator = !term->getAttrDictionary().empty() ||
+      printTerminator = !::mlir::isa<ImplicitTerminatorOpT>(*term) ||
+                        !term->getAttrDictionary().empty() ||
                         term->getNumOperands() != 0 ||
                         term->getNumResults() != 0;
     }

_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to