https://github.com/xlauko created https://github.com/llvm/llvm-project/pull/195618
Combine the separate `no_signed_wrap` and `no_unsigned_wrap` unit properties on arithmetic ops into a single `OverflowFlags` BitEnum (`nsw`, `nuw`). This allows combined flags to be written as `nsw|nuw` in assembly, replaces the per-flag verification traits with a single `OverflowFlagsRequireIntType` predicate, and folds the two `HasAtMostOneOfAttrs` checks into one `SatExclusiveWithOverflowFlags` predicate. The bit layout matches `mlir::LLVM::IntegerOverflowFlags`, so lowering casts the value directly and asserts the layout via static_assert. >From c957b87bae8edf4c1f965ebf830da396c562f170 Mon Sep 17 00:00:00 2001 From: xlauko <[email protected]> Date: Mon, 4 May 2026 11:03:32 +0200 Subject: [PATCH] [CIR] Replace nsw/nuw unit attrs with OverflowFlags BitEnum Combine the separate `no_signed_wrap` and `no_unsigned_wrap` unit properties on arithmetic ops into a single `OverflowFlags` BitEnum (`nsw`, `nuw`). This allows combined flags to be written as `nsw|nuw` in assembly, replaces the per-flag verification traits with a single `OverflowFlagsRequireIntType` predicate, and folds the two `HasAtMostOneOfAttrs` checks into one `SatExclusiveWithOverflowFlags` predicate. The bit layout matches `mlir::LLVM::IntegerOverflowFlags`, so lowering casts the value directly and asserts the layout via static_assert. --- .../CIR/Dialect/Builder/CIRBaseBuilder.h | 30 +- .../include/clang/CIR/Dialect/IR/CIRAttrs.td | 14 + .../clang/CIR/Dialect/IR/CIREnumAttr.td | 15 + clang/include/clang/CIR/Dialect/IR/CIROps.td | 73 ++- clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp | 8 +- .../TargetLowering/LowerItaniumCXXABI.cpp | 8 +- .../CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp | 22 +- clang/test/CIR/IR/invalid-overflow.cir | 39 ++ clang/test/CIR/IR/overflow-flags.cir | 66 ++ mlir/docs/ods-region-args-design.md | 614 ++++++++++++++++++ 10 files changed, 831 insertions(+), 58 deletions(-) create mode 100644 clang/test/CIR/IR/invalid-overflow.cir create mode 100644 clang/test/CIR/IR/overflow-flags.cir create mode 100644 mlir/docs/ods-region-args-design.md diff --git a/clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h b/clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h index 646fc7eb3c226..d3cc5a551fd27 100644 --- a/clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h +++ b/clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h @@ -61,6 +61,15 @@ constexpr bool testFlag(OverflowBehavior ob, OverflowBehavior flag) { return (ob & flag) != OverflowBehavior::None; } +inline OverflowFlags toOverflowFlags(OverflowBehavior ob) { + auto flags = OverflowFlags::none; + if (testFlag(ob, OverflowBehavior::NoSignedWrap)) + flags = flags | OverflowFlags::nsw; + if (testFlag(ob, OverflowBehavior::NoUnsignedWrap)) + flags = flags | OverflowFlags::nuw; + return flags; +} + class CIRBaseBuilderTy : public mlir::OpBuilder { public: @@ -279,18 +288,18 @@ class CIRBaseBuilderTy : public mlir::OpBuilder { } mlir::Value createInc(mlir::Location loc, mlir::Value input, - bool nsw = false) { - return cir::IncOp::create(*this, loc, input, nsw); + cir::OverflowFlags flags = cir::OverflowFlags::none) { + return cir::IncOp::create(*this, loc, input, flags); } mlir::Value createDec(mlir::Location loc, mlir::Value input, - bool nsw = false) { - return cir::DecOp::create(*this, loc, input, nsw); + cir::OverflowFlags flags = cir::OverflowFlags::none) { + return cir::DecOp::create(*this, loc, input, flags); } mlir::Value createMinus(mlir::Location loc, mlir::Value input, - bool nsw = false) { - return cir::MinusOp::create(*this, loc, input, nsw); + cir::OverflowFlags flags = cir::OverflowFlags::none) { + return cir::MinusOp::create(*this, loc, input, flags); } mlir::TypedAttr getConstPtrAttr(mlir::Type type, int64_t value) { @@ -629,8 +638,7 @@ class CIRBaseBuilderTy : public mlir::OpBuilder { mlir::Value createMul(mlir::Location loc, mlir::Value lhs, mlir::Value rhs, OverflowBehavior ob = OverflowBehavior::None) { auto op = cir::MulOp::create(*this, loc, lhs, rhs); - op.setNoUnsignedWrap(testFlag(ob, OverflowBehavior::NoUnsignedWrap)); - op.setNoSignedWrap(testFlag(ob, OverflowBehavior::NoSignedWrap)); + op.setFlags(toOverflowFlags(ob)); return op; } mlir::Value createNSWMul(mlir::Location loc, mlir::Value lhs, @@ -645,8 +653,7 @@ class CIRBaseBuilderTy : public mlir::OpBuilder { mlir::Value createSub(mlir::Location loc, mlir::Value lhs, mlir::Value rhs, OverflowBehavior ob = OverflowBehavior::None) { auto op = cir::SubOp::create(*this, loc, lhs, rhs); - op.setNoUnsignedWrap(testFlag(ob, OverflowBehavior::NoUnsignedWrap)); - op.setNoSignedWrap(testFlag(ob, OverflowBehavior::NoSignedWrap)); + op.setFlags(toOverflowFlags(ob)); op.setSaturated(testFlag(ob, OverflowBehavior::Saturated)); return op; } @@ -664,8 +671,7 @@ class CIRBaseBuilderTy : public mlir::OpBuilder { mlir::Value createAdd(mlir::Location loc, mlir::Value lhs, mlir::Value rhs, OverflowBehavior ob = OverflowBehavior::None) { auto op = cir::AddOp::create(*this, loc, lhs, rhs); - op.setNoUnsignedWrap(testFlag(ob, OverflowBehavior::NoUnsignedWrap)); - op.setNoSignedWrap(testFlag(ob, OverflowBehavior::NoSignedWrap)); + op.setFlags(toOverflowFlags(ob)); op.setSaturated(testFlag(ob, OverflowBehavior::Saturated)); return op; } diff --git a/clang/include/clang/CIR/Dialect/IR/CIRAttrs.td b/clang/include/clang/CIR/Dialect/IR/CIRAttrs.td index 1520999e3f85f..703d75bb2c14c 100644 --- a/clang/include/clang/CIR/Dialect/IR/CIRAttrs.td +++ b/clang/include/clang/CIR/Dialect/IR/CIRAttrs.td @@ -110,6 +110,20 @@ def CIR_SourceLanguageAttr : CIR_EnumAttr<CIR_SourceLanguage, "lang"> { }]; } +//===----------------------------------------------------------------------===// +// OverflowFlagsAttr +//===----------------------------------------------------------------------===// + +def CIR_OFnone : I32BitEnumCaseNone<"none">; +def CIR_OFnsw : I32BitEnumCaseBit<"nsw", 0>; +def CIR_OFnuw : I32BitEnumCaseBit<"nuw", 1>; + +def CIR_OverflowFlags : CIR_I32BitEnum< + "OverflowFlags", "integer arithmetic overflow flags", + [CIR_OFnone, CIR_OFnsw, CIR_OFnuw]>; + +def CIR_OverflowFlagsProp : CIR_BitEnumProp<CIR_OverflowFlags>; + //===----------------------------------------------------------------------===// // ArgPassingKind + RecordLayoutAttr //===----------------------------------------------------------------------===// diff --git a/clang/include/clang/CIR/Dialect/IR/CIREnumAttr.td b/clang/include/clang/CIR/Dialect/IR/CIREnumAttr.td index 1de6ffdc08d72..78f74ec8393ae 100644 --- a/clang/include/clang/CIR/Dialect/IR/CIREnumAttr.td +++ b/clang/include/clang/CIR/Dialect/IR/CIREnumAttr.td @@ -31,6 +31,21 @@ class CIR_EnumAttr<EnumAttrInfo info, string name = "", list<Trait> traits = []> let assemblyFormat = "`<` $value `>`"; } +class CIR_I32BitEnum<string name, string summary, + list<BitEnumCaseBase> cases> + : I32BitEnum<name, summary, cases> { + let cppNamespace = "::cir"; + let separator = "|"; + let printBitEnumPrimaryGroups = 1; +} + +// CIR property wrapping a BitEnum. Storage is inline (Property, not Attribute); +// the auto-generated FieldParser parses bare `flag1|flag2` syntax. +class CIR_BitEnumProp<BitEnum info, string defaultVal = info.cppType # "::none"> + : EnumProp<info> { + let defaultValue = defaultVal; +} + class CIR_DefaultValuedEnumParameter<EnumAttrInfo info, string value = ""> : EnumParameter<info> { let defaultValue = value; diff --git a/clang/include/clang/CIR/Dialect/IR/CIROps.td b/clang/include/clang/CIR/Dialect/IR/CIROps.td index 46b23c32b1a98..4ad247a8dba68 100644 --- a/clang/include/clang/CIR/Dialect/IR/CIROps.td +++ b/clang/include/clang/CIR/Dialect/IR/CIROps.td @@ -130,10 +130,22 @@ class FlagRequiresIntType<string flag> : PredOpTrait< CPred<"::mlir::isa<::cir::IntType>(this->getResult().getType())">]> >; -def NSWFlagIntOnly : FlagRequiresIntType<"no_signed_wrap">; -def NUWFlagIntOnly : FlagRequiresIntType<"no_unsigned_wrap">; def SatFlagIntOnly : FlagRequiresIntType<"saturated">; +// Requires that overflow flags imply an integer result type. +def OverflowFlagsRequireIntType : PredOpTrait< + "overflow flags require an integer result type", + Or<[CPred<"this->getFlags() == ::cir::OverflowFlags::none">, + CPred<"::mlir::isa<::cir::IntType>(this->getResult().getType())">]> +>; + +// Requires that the saturated flag is mutually exclusive with overflow flags. +def SatExclusiveWithOverflowFlags : PredOpTrait< + "sat is mutually exclusive with nsw/nuw overflow flags", + CPred<"!this->getSaturated() || " + "this->getFlags() == ::cir::OverflowFlags::none"> +>; + //===----------------------------------------------------------------------===// // CastOp //===----------------------------------------------------------------------===// @@ -1831,12 +1843,12 @@ class CIR_UnaryOpWithOverflowFlag<string mnemonic, Type type, list<Trait> traits = []> : CIR_UnaryOp<mnemonic, type, traits> { - let append traits = [NSWFlagIntOnly]; + let append traits = [OverflowFlagsRequireIntType]; - let append arguments = (ins UnitProp:$no_signed_wrap); + let append arguments = (ins CIR_OverflowFlagsProp:$flags); let prepend assemblyFormat = [{ - (`nsw` $no_signed_wrap^)? + ($flags^)? }]; } @@ -2383,16 +2395,12 @@ class CIR_BinaryOp<string mnemonic, Type type, list<Trait> traits = []> class CIR_BinaryOpWithOverflowFlags<string mnemonic, Type type> : CIR_BinaryOp<mnemonic, type> { - let append traits = [Pure, NSWFlagIntOnly, NUWFlagIntOnly]; + let append traits = [Pure, OverflowFlagsRequireIntType]; - let append arguments = (ins - UnitProp:$no_signed_wrap, - UnitProp:$no_unsigned_wrap - ); + let append arguments = (ins CIR_OverflowFlagsProp:$flags); let prepend assemblyFormat = [{ - (`nsw` $no_signed_wrap^)? - (`nuw` $no_unsigned_wrap^)? + ($flags^)? }]; } @@ -2400,11 +2408,7 @@ class CIR_BinaryOpWithOverflowFlags<string mnemonic, Type type> class CIR_SaturatableBinaryOp<string mnemonic, Type type> : CIR_BinaryOpWithOverflowFlags<mnemonic, type> { - let append traits = [ - SatFlagIntOnly, - HasAtMostOneOfAttrs<["saturated", "no_signed_wrap"]>, - HasAtMostOneOfAttrs<["saturated", "no_unsigned_wrap"]> - ]; + let append traits = [SatFlagIntOnly, SatExclusiveWithOverflowFlags]; let append arguments = (ins UnitProp:$saturated); @@ -2426,19 +2430,21 @@ def CIR_AddOp : CIR_SaturatableBinaryOp<"add", CIR_AnyArithType> { operands. Both operands and the result must have the same type. For integer types, the optional `nsw` (no signed wrap) and `nuw` (no - unsigned wrap) unit attributes indicate that the result is poison if signed - or unsigned overflow occurs, respectively. The optional `sat` (saturated) - attribute clamps the result to the type's representable range instead of - wrapping. The `nsw`/`nuw` flags and `sat` are mutually exclusive. - + unsigned wrap) overflow flags indicate that the result is poison if signed + or unsigned overflow occurs, respectively. Combined flags are written as + `nsw|nuw`. The optional `sat` (saturated) attribute clamps the result to + the type's representable range instead of wrapping. The `nsw`/`nuw` flags + and `sat` are mutually exclusive. + Example: ``` %0 = cir.add %a, %b : !s32i %1 = cir.add nsw %a, %b : !s32i %2 = cir.add nuw %a, %b : !u32i - %3 = cir.add sat %a, %b : !s32i - %4 = cir.add %a, %b : !cir.float + %3 = cir.add nsw|nuw %a, %b : !s32i + %4 = cir.add sat %a, %b : !s32i + %5 = cir.add %a, %b : !cir.float ``` }]; } @@ -2454,11 +2460,12 @@ def CIR_SubOp : CIR_SaturatableBinaryOp<"sub", CIR_AnyArithType> { operands. Both operands and the result must have the same type. For integer types, the optional `nsw` (no signed wrap) and `nuw` (no - unsigned wrap) unit attributes indicate that the result is poison if signed - or unsigned overflow occurs, respectively. The optional `sat` (saturated) - attribute clamps the result to the type's representable range. The - `nsw`/`nuw` flags and `sat` are mutually exclusive. - + unsigned wrap) overflow flags indicate that the result is poison if signed + or unsigned overflow occurs, respectively. Combined flags are written as + `nsw|nuw`. The optional `sat` (saturated) attribute clamps the result to + the type's representable range. The `nsw`/`nuw` flags and `sat` are + mutually exclusive. + Example: ``` @@ -2483,8 +2490,9 @@ def CIR_MulOp : CIR_BinaryOpWithOverflowFlags<"mul", CIR_AnyArithType> { operands. Both operands and the result must have the same type. For integer types, the optional `nsw` (no signed wrap) and `nuw` (no - unsigned wrap) unit attributes indicate that the result is poison if signed - or unsigned overflow occurs, respectively. + unsigned wrap) overflow flags indicate that the result is poison if signed + or unsigned overflow occurs, respectively. Combined flags are written as + `nsw|nuw`. Example: @@ -2492,7 +2500,8 @@ def CIR_MulOp : CIR_BinaryOpWithOverflowFlags<"mul", CIR_AnyArithType> { %0 = cir.mul %a, %b : !s32i %1 = cir.mul nsw %a, %b : !s32i %2 = cir.mul nuw %a, %b : !u32i - %3 = cir.mul %a, %b : !cir.float + %3 = cir.mul nsw|nuw %a, %b : !s32i + %4 = cir.mul %a, %b : !cir.float ``` }]; } diff --git a/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp b/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp index 92b7156f3a3a8..07a160b4aa3e0 100644 --- a/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp +++ b/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp @@ -796,16 +796,18 @@ class ScalarExprEmitter : public StmtVisitor<ScalarExprEmitter, mlir::Value> { // NOTE: LLVM codegen will lower this directly to either a FNeg // or a Sub instruction. In CIR this will be handled later in LowerToLLVM. + auto flags = nsw ? cir::OverflowFlags::nsw : cir::OverflowFlags::none; return builder.createOrFold<cir::MinusOp>( - cgf.getLoc(e->getSourceRange().getBegin()), operand, nsw); + cgf.getLoc(e->getSourceRange().getBegin()), operand, flags); } mlir::Value emitIncOrDec(const UnaryOperator *e, mlir::Value input, bool nsw = false) { mlir::Location loc = cgf.getLoc(e->getSourceRange().getBegin()); + auto flags = nsw ? cir::OverflowFlags::nsw : cir::OverflowFlags::none; return e->isIncrementOp() - ? builder.createOrFold<cir::IncOp>(loc, input, nsw) - : builder.createOrFold<cir::DecOp>(loc, input, nsw); + ? builder.createOrFold<cir::IncOp>(loc, input, flags) + : builder.createOrFold<cir::DecOp>(loc, input, flags); } mlir::Value VisitUnaryNot(const UnaryOperator *e) { diff --git a/clang/lib/CIR/Dialect/Transforms/TargetLowering/LowerItaniumCXXABI.cpp b/clang/lib/CIR/Dialect/Transforms/TargetLowering/LowerItaniumCXXABI.cpp index 8769975bdc948..cc2d39116181f 100644 --- a/clang/lib/CIR/Dialect/Transforms/TargetLowering/LowerItaniumCXXABI.cpp +++ b/clang/lib/CIR/Dialect/Transforms/TargetLowering/LowerItaniumCXXABI.cpp @@ -432,11 +432,11 @@ static mlir::Value lowerDataMemberCast(mlir::Operation *op, mlir::Value adjustedPtr; if (isDerivedToBase) { auto subOp = cir::SubOp::create(builder, loc, ty, loweredSrc, offsetValue); - subOp.setNoSignedWrap(true); + subOp.setFlags(cir::OverflowFlags::nsw); adjustedPtr = subOp; } else { auto addOp = cir::AddOp::create(builder, loc, ty, loweredSrc, offsetValue); - addOp.setNoSignedWrap(true); + addOp.setFlags(cir::OverflowFlags::nsw); adjustedPtr = addOp; } @@ -477,12 +477,12 @@ static mlir::Value lowerMethodCast(mlir::Operation *op, mlir::Value loweredSrc, if (isDerivedToBase) { auto subOp = cir::SubOp::create(builder, op->getLoc(), ptrdiffCIRTy, adjField, offsetValue); - subOp.setNoSignedWrap(true); + subOp.setFlags(cir::OverflowFlags::nsw); adjustedAdjField = subOp; } else { auto addOp = cir::AddOp::create(builder, op->getLoc(), ptrdiffCIRTy, adjField, offsetValue); - addOp.setNoSignedWrap(true); + addOp.setFlags(cir::OverflowFlags::nsw); adjustedAdjField = addOp; } diff --git a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp index e17c7a209db6b..614a534dce232 100644 --- a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp +++ b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp @@ -2582,7 +2582,8 @@ lowerIncDecOp(CIROp op, typename CIROp::Adaptor adaptor, mlir::Location loc = op.getLoc(); if (mlir::isa<cir::IntType>(elementType)) { - auto maybeNSW = nswFlag(op.getNoSignedWrap()); + auto maybeNSW = + nswFlag(bitEnumContainsAll(op.getFlags(), cir::OverflowFlags::nsw)); auto one = mlir::LLVM::ConstantOp::create(rewriter, loc, llvmType, 1); rewriter.replaceOpWithNewOp<LLVMIntOp>(op, adaptor.getInput(), one, maybeNSW); @@ -2621,7 +2622,8 @@ mlir::LogicalResult CIRToLLVMMinusOpLowering::matchAndRewrite( mlir::Location loc = op.getLoc(); if (mlir::isa<cir::IntType>(elementType)) { - auto maybeNSW = nswFlag(op.getNoSignedWrap()); + auto maybeNSW = + nswFlag(bitEnumContainsAll(op.getFlags(), cir::OverflowFlags::nsw)); mlir::Value zero; if (isVector) zero = mlir::LLVM::ZeroOp::create(rewriter, loc, llvmType); @@ -2683,11 +2685,17 @@ static bool isIntTypeUnsigned(mlir::Type type) { template <typename BinOp> static mlir::LLVM::IntegerOverflowFlags intOverflowFlag(BinOp op) { - if (op.getNoUnsignedWrap()) - return mlir::LLVM::IntegerOverflowFlags::nuw; - if (op.getNoSignedWrap()) - return mlir::LLVM::IntegerOverflowFlags::nsw; - return mlir::LLVM::IntegerOverflowFlags::none; + // cir::OverflowFlags and mlir::LLVM::IntegerOverflowFlags share the same + // bit positions (nsw=0, nuw=1) so the underlying values map directly. + static_assert( + static_cast<uint32_t>(cir::OverflowFlags::nsw) == + static_cast<uint32_t>(mlir::LLVM::IntegerOverflowFlags::nsw), + "nsw bit position mismatch between cir and llvm dialects"); + static_assert( + static_cast<uint32_t>(cir::OverflowFlags::nuw) == + static_cast<uint32_t>(mlir::LLVM::IntegerOverflowFlags::nuw), + "nuw bit position mismatch between cir and llvm dialects"); + return static_cast<mlir::LLVM::IntegerOverflowFlags>(op.getFlags()); } /// Lower an arithmetic op that supports saturation, overflow flags, and an FP diff --git a/clang/test/CIR/IR/invalid-overflow.cir b/clang/test/CIR/IR/invalid-overflow.cir new file mode 100644 index 0000000000000..e92dd8b0f6a43 --- /dev/null +++ b/clang/test/CIR/IR/invalid-overflow.cir @@ -0,0 +1,39 @@ +// RUN: cir-opt %s -verify-diagnostics -split-input-file + +!s32i = !cir.int<s, 32> + +cir.func @nsw_on_float(%a: !cir.float, %b: !cir.float) { + // expected-error @below {{overflow flags require an integer result type}} + %0 = cir.add nsw %a, %b : !cir.float + cir.return +} + +// ----- + +!s32i = !cir.int<s, 32> + +cir.func @nuw_on_float(%a: !cir.float, %b: !cir.float) { + // expected-error @below {{overflow flags require an integer result type}} + %0 = cir.mul nuw %a, %b : !cir.float + cir.return +} + +// ----- + +!s32i = !cir.int<s, 32> + +cir.func @sat_with_nsw(%a: !s32i, %b: !s32i) { + // expected-error @below {{sat is mutually exclusive with nsw/nuw overflow flags}} + %0 = cir.add sat nsw %a, %b : !s32i + cir.return +} + +// ----- + +!s32i = !cir.int<s, 32> + +cir.func @sat_with_nuw(%a: !s32i, %b: !s32i) { + // expected-error @below {{sat is mutually exclusive with nsw/nuw overflow flags}} + %0 = cir.sub sat nuw %a, %b : !s32i + cir.return +} diff --git a/clang/test/CIR/IR/overflow-flags.cir b/clang/test/CIR/IR/overflow-flags.cir new file mode 100644 index 0000000000000..78ef1969e9f59 --- /dev/null +++ b/clang/test/CIR/IR/overflow-flags.cir @@ -0,0 +1,66 @@ +// RUN: cir-opt %s --verify-roundtrip | FileCheck %s + +!s32i = !cir.int<s, 32> +!u32i = !cir.int<u, 32> + +module { + // Round-trip every BitEnum state on cir.add: none (default, elided), + // nsw alone, nuw alone, both flags combined, plus saturated. + cir.func @test_add_flags(%a: !s32i, %b: !s32i, %c: !u32i, %d: !u32i) { + %0 = cir.add %a, %b : !s32i + %1 = cir.add nsw %a, %b : !s32i + %2 = cir.add nuw %c, %d : !u32i + %3 = cir.add nsw|nuw %a, %b : !s32i + %4 = cir.add sat %a, %b : !s32i + cir.return + } + // CHECK-LABEL: cir.func{{.*}} @test_add_flags + // CHECK: cir.add %{{.+}}, %{{.+}} : !s32i + // CHECK: cir.add nsw %{{.+}}, %{{.+}} : !s32i + // CHECK: cir.add nuw %{{.+}}, %{{.+}} : !u32i + // CHECK: cir.add nsw|nuw %{{.+}}, %{{.+}} : !s32i + // CHECK: cir.add sat %{{.+}}, %{{.+}} : !s32i + + cir.func @test_sub_flags(%a: !s32i, %b: !s32i) { + %0 = cir.sub %a, %b : !s32i + %1 = cir.sub nsw %a, %b : !s32i + %2 = cir.sub nsw|nuw %a, %b : !s32i + %3 = cir.sub sat %a, %b : !s32i + cir.return + } + // CHECK-LABEL: cir.func{{.*}} @test_sub_flags + // CHECK: cir.sub %{{.+}}, %{{.+}} : !s32i + // CHECK: cir.sub nsw %{{.+}}, %{{.+}} : !s32i + // CHECK: cir.sub nsw|nuw %{{.+}}, %{{.+}} : !s32i + // CHECK: cir.sub sat %{{.+}}, %{{.+}} : !s32i + + cir.func @test_mul_flags(%a: !s32i, %b: !s32i) { + %0 = cir.mul %a, %b : !s32i + %1 = cir.mul nsw %a, %b : !s32i + %2 = cir.mul nuw %a, %b : !s32i + %3 = cir.mul nsw|nuw %a, %b : !s32i + cir.return + } + // CHECK-LABEL: cir.func{{.*}} @test_mul_flags + // CHECK: cir.mul %{{.+}}, %{{.+}} : !s32i + // CHECK: cir.mul nsw %{{.+}}, %{{.+}} : !s32i + // CHECK: cir.mul nuw %{{.+}}, %{{.+}} : !s32i + // CHECK: cir.mul nsw|nuw %{{.+}}, %{{.+}} : !s32i + + cir.func @test_unary_flags(%a: !s32i) { + %0 = cir.minus %a : !s32i + %1 = cir.minus nsw %a : !s32i + %2 = cir.inc %a : !s32i + %3 = cir.inc nsw %a : !s32i + %4 = cir.dec %a : !s32i + %5 = cir.dec nsw %a : !s32i + cir.return + } + // CHECK-LABEL: cir.func{{.*}} @test_unary_flags + // CHECK: cir.minus %{{.+}} : !s32i + // CHECK: cir.minus nsw %{{.+}} : !s32i + // CHECK: cir.inc %{{.+}} : !s32i + // CHECK: cir.inc nsw %{{.+}} : !s32i + // CHECK: cir.dec %{{.+}} : !s32i + // CHECK: cir.dec nsw %{{.+}} : !s32i +} diff --git a/mlir/docs/ods-region-args-design.md b/mlir/docs/ods-region-args-design.md new file mode 100644 index 0000000000000..2f7d0d56fbeef --- /dev/null +++ b/mlir/docs/ods-region-args-design.md @@ -0,0 +1,614 @@ +--- +name: ODS Region Args Design +description: Design doc for built-in ODS format directives for region entry arguments (PR #190545 redesign). Covers directives, examples for all surveyed ops, multi-PR development plan. +type: project +--- + +# Design: Region Entry Arguments in ODS Assembly Format + +## Context + +PR #190545 adds `stashRegionArguments`/`takeRegionArguments` to `OpAsmParser`. Mehdi's +review asks for a more general, composable, ODS-driven approach that covers both +function-like and loop-like ops. This design aims to eliminate `custom<>` for region +arg handling entirely, using only built-in ODS format directives. + +**Why:** Current ODS format strings cannot express region entry arguments parsed +separately from the region body. This forces all ops with region args (function-like, +loop-like) to use `hasCustomAssemblyFormat = 1` with hand-written parsers. + +**How to apply:** This is the active design for llvm/llvm-project#190545. Use this +as reference when implementing the PR stack. + +## Survey: All Region Arg Patterns Across Projects + +### Three fundamental patterns (across MLIR, CIRCT, and out-of-tree projects) + +**Pattern 1 — Typed arg list** (function-like): `(%arg0 : type0, %arg1 : type1 {attrs})` +- MLIR: `func.func`, `async.execute` +- CIRCT: `hw.module`, `firrtl.module`, `calyx.component`, `arc.define`, `sv.func` +- Other out-of-tree: any `FunctionOpInterface` impl (common pattern) +- CIR: `cir.func` + +**Pattern 2 — Assignment list** (while-like): `(%arg0 = %init0, %arg1 = %init1)` +- MLIR: `scf.while`, `scf.for` iter_args, `scf.forall` shared_outs, `affine.for` iter_args + +**Pattern 3 — Single interleaved arg** (for-like): `%iv = %lb to %ub step %step` +- MLIR: `scf.for`, `scf.forall`, `scf.parallel`, `affine.for` +- CIRCT: `sv.for` + +All three patterns ultimately produce `SmallVector<OpAsmParser::Argument>` and pass it +to `parseRegion()`. The existing `OpAsmParser` already has all needed parse methods: +- `parseArgument(arg, allowType, allowAttrs)` +- `parseArgumentList(args, delimiter, allowType, allowAttrs)` +- `parseAssignmentList(lhs, rhs)` / `parseOptionalAssignmentList(lhs, rhs)` +- `parseRegion(region, arguments)` + +The only missing piece is ODS format syntax to connect them. + +--- + +## New Built-in ODS Directives + +### Mechanism + +The format generator maintains a `SmallVector<OpAsmParser::Argument>` per region as a +**local variable** in the generated `parse()` method. New format directives **accumulate** +arguments into this vector. When `$region` is encountered, the generator emits +`parseRegion(region, accumulatedArgs)` instead of `parseRegion(region)`. + +On the print side, `$region` emits `printRegion(region, /*printEntryBlockArgs=*/false)` +when args have been accumulated, since the arg-producing directives handle printing. + +**No changes to `OpAsmParser` or `OpAsmPrinter`.** + +### Directive 1: `region-arg($region)` + +Parses a single **typed** region argument: `%name : type`. + +``` +Parse: parser.parseArgument(arg, /*allowType=*/true) + {region}RegionArgs.push_back(arg); +Print: printer.printRegionArgument(region.getArgument(N)) +``` + +Usage: `region-arg($body)` anywhere in a format string. +Multiple `region-arg($body)` directives accumulate in order. + +### Directive 2: `region-arg($region, type-of=$operand)` + +Parses a single region argument **name only** (no type). Type is inferred from `$operand` +after operand type resolution. + +``` +Parse: parser.parseArgument(arg, /*allowType=*/false) + // deferred: arg.type = resolvedTypeOf($operand) + {region}RegionArgs.push_back(arg); +Print: printer << region.getArgument(N).getName() // name only, type printed elsewhere +``` + +Usage: `region-arg($body, type-of=$lowerBound)` — for loop induction variables whose +type matches a bound operand. + +`type-of=$operand` creates a deferred type assignment: after all types are resolved, +the generator emits `{region}RegionArgs[N].type = {operand}Type;`. + +### Directive 3: `region-args($region)` + +Parses a comma-separated list of **typed** region arguments: `%a : t0, %b : t1`. + +``` +Parse: parser.parseArgumentList(args, Delimiter::None, /*allowType=*/true) + {region}RegionArgs.append(args); +Print: interleaveComma(region.getArguments(), [](arg) { + printer.printRegionArgument(arg); + }) +``` + +Does NOT parse argument attributes. For args with attrs, use `function-signature`. + +### Directive 4: `region-args($region, type-of=$operands)` + +Parses a comma-separated list of region argument **names only**. Types are inferred +from the variadic `$operands` after type resolution. + +``` +Parse: parser.parseArgumentList(args, Delimiter::None, /*allowType=*/false) + // deferred: for each (arg, op) in zip(args, operands): arg.type = op.type + {region}RegionArgs.append(args); +Print: interleaveComma(region.getArguments(), [](arg) { + printer << arg; // name only + }) +``` + +Usage: `region-args($body, type-of=$lowerBound)` — for parallel loop induction +variables whose types match bound operands. + +### Directive 5: `assignment-list(region-args($region), $operands)` + +Parses an assignment list: `%arg0 = %init0, %arg1 = %init1`. + +``` +Parse: parser.parseAssignmentList({region}MoreArgs, {operands}Operands) + {region}RegionArgs.append({region}MoreArgs); + // deferred: for each (arg, op) in zip(moreArgs, operands): arg.type = op.type +Print: printInitializationList(printer, region.getArguments().drop_front(N), + operandValues, "") +``` + +Produces both region args AND operand values. Types inferred from operands. + +### Directive 6: `function-signature(region-args($region), $function_type)` + +Parses a full function signature: `(%arg0 : type0 {attrs}, ...) -> (result_types)`. + +``` +Parse: function_interface_impl::parseFunctionSignature(parser, args, argTypes, resultTypes) + {region}RegionArgs.append(args); + $function_type = FunctionType::get(ctx, argTypes, resultTypes); + call_interface_impl::addArgAndResultAttrs(builder, result, args, resultAttrs, + argAttrsName, resAttrsName); +Print: function_interface_impl::printFunctionSignature(printer, op, argTypes, ...) +``` + +This is the directive that handles the **full arg-attrs complexity**. It wraps the +existing `function_interface_impl` helpers and is the only directive that interacts +with argument attributes. + +### Argument Attributes — Detailed Interaction + +The `OpAsmParser::Argument` struct carries attrs: +```cpp +struct Argument { + UnresolvedOperand ssaName; + Type type; + DictionaryAttr attrs; // per-arg attributes + std::optional<Location> sourceLoc; +}; +``` + +The existing parse APIs support attrs via opt-in flags: +```cpp +parseArgument(arg, allowType, allowAttrs); +parseArgumentList(args, delimiter, allowType, allowAttrs); +``` + +When enabled, the syntax becomes: `%arg0 : type {some.attr = 42} loc("file":1:2)`. + +**Where attrs are stored** depends on the op kind: + +| Storage mechanism | When used | Example ops | +|------------------|-----------|-------------| +| `FunctionOpInterface` `arg_attrs`/`res_attrs` | Function-like ops | `func.func`, `cir.func`, `gpu.func`, `hw.module` | +| (Ignored — `parseRegion` discards `Argument.attrs`) | Non-function ops | `scf.for`, `scf.while`, loop IVs | + +**Per-directive behavior:** + +| Directive | Parses attrs? | Stores attrs? | Why | +|-----------|:---:|:---:|-----| +| `function-signature` | **Yes** | **Yes** — via `addArgAndResultAttrs` | Function args carry attrs | +| `region-args($r)` | No | N/A | Simple typed arg lists. If attrs needed, use `function-signature`. | +| `region-arg($r)` | No | N/A | Loop IVs — never have attrs | +| `region-arg($r, type-of=$op)` | No | N/A | Loop IVs with inferred type — never have attrs | +| `assignment-list(...)` | No | N/A | `parseAssignmentList` API does not support attrs on LHS | + +**Design rationale**: Argument attributes are exclusively a function-like op concern. +The `function-signature` directive encapsulates ALL the complexity: + +1. `parseFunctionArgumentList` calls `parseOptionalArgument(arg, allowType=true, allowAttrs=true)` +2. Each arg's `DictionaryAttr attrs` is populated by the parser +3. `addArgAndResultAttrs(builder, result, entryArgs, resultAttrs, ...)` stores per-arg + attrs as the op's `arg_attrs` array attribute +4. On print, `printFunctionSignature` reads attrs from `FunctionOpInterface::getArgAttrs(i)` + and passes them to `printRegionArgument(blockArg, argAttrs)` +5. `printFunctionAttributes` elides `arg_attrs`/`res_attrs` from `attr-dict` + +Non-function directives (`region-arg`, `region-args`, `assignment-list`) call +`parseArgument`/`parseArgumentList` with `allowAttrs=false`. The parsed `Argument.attrs` +is always empty. `parseRegion` receives these args and creates block arguments using +only `ssaName` and `type` — it ignores the `attrs` field regardless. + +**If a future non-function op needs arg attrs**: the op would need to declare +attr storage (e.g., an `arg_attrs` array property). A new directive variant like +`region-args($r, allow_attrs)` could be added, with the generator emitting attr +storage code. This is not needed today — no upstream non-function op uses arg attrs. + +### Enhanced `$region` directive + +When the region has accumulated args from any of the above directives: +``` +Parse: parser.parseRegion(*region, {region}RegionArgs) +Print: printer.printRegion(region, /*printEntryBlockArgs=*/false, ...) +``` + +When no args accumulated: behavior unchanged. + +--- + +## ODS Format Examples: All Surveyed Operations (No `custom<>`) + +### `func.func` + +```tablegen +let assemblyFormat = [{ + $sym_name function-signature(region-args($body), $function_type) + attr-dict-with-keyword $body +}]; +``` + +### `scf.for` + +```tablegen +let assemblyFormat = [{ + (`unsigned` $unsignedCmp^)? + region-arg($body, type-of=$lowerBound) `=` $lowerBound `to` $upperBound `step` $step + (`iter_args` `(` assignment-list(region-args($body), $initArgs) `)` `->` `(` type(results) `)`)? + (`:` type($lowerBound)^)? + $body attr-dict +}]; +``` + +### `scf.while` + +```tablegen +let assemblyFormat = [{ + (`(` assignment-list(region-args($before), $inits) `)`)? + `:` functional-type(type($inits), type(results)) + $before `do` $after attr-dict +}]; +``` + +### `scf.if` + +```tablegen +let assemblyFormat = [{ + $condition (`->` `(` type(results) `)`)? $thenRegion (`else` $elseRegion)? attr-dict +}]; +``` + +No region args. Already expressible today. + +### `scf.forall` + +```tablegen +let assemblyFormat = [{ + `(` region-args($body, type-of=$lowerBound) `)` `=` + `(` $lowerBound `)` `to` `(` $upperBound `)` `step` `(` $step `)` + (`shared_outs` `(` assignment-list(region-args($body), $outputs) `)` `->` `(` type(results) `)`)? + $body (`mapping` $mapping^)? attr-dict +}]; +``` + +### `scf.parallel` + +```tablegen +let assemblyFormat = [{ + `(` region-args($body, type-of=$lowerBound) `)` `=` + `(` $lowerBound `)` `to` `(` $upperBound `)` `step` `(` $step `)` + (`init` `(` assignment-list(region-args($body), $initVals) `)` + `->` type(results))? + $body attr-dict +}]; +``` + +### `affine.for` + +```tablegen +let assemblyFormat = [{ + region-arg($body, type=index) `=` + affine-bound($lowerBoundMap, $lowerBoundOperands) `to` + affine-bound($upperBoundMap, $upperBoundOperands) + (`step` $step^)? + (`iter_args` `(` assignment-list(region-args($body), $initArgs) `)` `->` `(` type(results) `)`)? + $body attr-dict +}]; +``` + +### `async.execute` + +```tablegen +let assemblyFormat = [{ + (`[` $dependencies `]`)? + (`(` assignment-list(region-args($bodyRegion), $bodyOperands) + `:` type($bodyOperands) `)`)? + `->` type(results) $bodyRegion attr-dict +}]; +``` + +### `gpu.func` + +```tablegen +let assemblyFormat = [{ + $sym_name function-signature(region-args($body), $function_type) + (`workgroup` `(` region-args($body) `)`)? + (`private` `(` region-args($body) `)`)? + attr-dict-with-keyword $body +}]; +``` + +### `cir.func` + +```tablegen +let assemblyFormat = [{ + (`builtin` $builtin^)? + (`coroutine` $coroutine^)? + ($inline_kind^)? + (`lambda` $lambda^)? + (`no_proto` $no_proto^)? + (`comdat` $comdat^)? + $linkage $visibility + $sym_name function-signature(region-args($body), $function_type) + (`alias` `(` $aliasee `)`^)? + (`extra` `(` $extra_attrs `)`^)? + attr-dict-with-keyword $body +}]; +``` + +### CIRCT: `hw.module` + +```tablegen +let assemblyFormat = [{ + $sym_name function-signature(region-args($body), $module_type) + attr-dict-with-keyword $body +}]; +``` + +### CIRCT: `sv.for` + +```tablegen +let assemblyFormat = [{ + region-arg($body) `=` $lowerBound `to` $upperBound `step` $step + $body attr-dict +}]; +``` + +--- + +## Directive Usage Summary + +| Directive | Ops using it | +|-----------|-------------| +| `region-arg($r)` | `sv.for` | +| `region-arg($r, type-of=$op)` | `scf.for` (IV), `scf.forall` (IVs), `scf.parallel` (IVs) | +| `region-arg($r, type=T)` | `affine.for` (IV, always index) | +| `region-args($r)` | `gpu.func` (attributions) | +| `assignment-list(region-args($r), $ops)` | `scf.while`, `scf.for` iter_args, `scf.forall` shared_outs, `scf.parallel` init, `affine.for` iter_args, `async.execute` | +| `function-signature(region-args($r), $ft)` | `func.func`, `cir.func`, `gpu.func`, `hw.module`, `firrtl.module`, `calyx.component`, `arc.define`, `sv.func` | +| No region args needed | `scf.if`, `affine.if`, `cir.if`, `cir.for`, `cir.while` | + +--- + +## Multi-Stage Development Plan + +### PR Stack + +``` +main + +- PR 1: [MLIR][ODS] Core region-arg accumulation + region-arg/region-args directives + +- PR 2: [MLIR][ODS] assignment-list(region-args) directive + +- PR 3: [MLIR][ODS] region-arg type-of deferred type resolution + | +- PR 5: [SCF] Migrate scf.for to ODS assembly format + | +- PR 7: [SCF] Migrate scf.forall/scf.parallel to ODS assembly format + +- PR 4: [SCF] Migrate scf.while to ODS assembly format + +- PR 6: [Async] Migrate async.execute to ODS assembly format + +- PR 8: [MLIR][ODS] function-signature directive + +- PR 9: [Func] Migrate func.func to ODS assembly format + +- PR 10: [CIR] Migrate cir.func to ODS assembly format +``` + +### PR 1: Core region-arg accumulation + `region-arg` / `region-args` + +**Goal**: Establish the fundamental mechanism in OpFormatGen. + +**Files:** +- `mlir/tools/mlir-tblgen/OpFormatGen.cpp` — New format element classes, per-region + local variable tracking, enhanced `$region` codegen, format string parser. +- `mlir/test/lib/Dialect/Test/TestOpsSyntax.td` — Test ops. +- `mlir/test/mlir-tblgen/op-format.mlir` — Round-trip tests. + +**Scope**: ~300-400 lines. No changes to OpAsmParser. + +### PR 2: `assignment-list(region-args($region), $operands)` + +**Goal**: Support the while-pattern (`%arg = %init` syntax). + +**Depends on**: PR 1. **Scope**: ~200 lines. + +### PR 3: `region-arg($region, type-of=$operand)` deferred type resolution + +**Goal**: Support the for-pattern (region arg type inferred from operand). + +**Depends on**: PR 1. **Scope**: ~150 lines. + +### PR 4: Migrate `scf.while` to ODS assembly format + +**Depends on**: PR 2. Removes ~60 lines of hand-written parser/printer. + +### PR 5: Migrate `scf.for` to ODS assembly format + +**Depends on**: PR 2 + PR 3. Removes ~100 lines. + +### PR 6: Migrate `async.execute` to ODS assembly format + +**Depends on**: PR 2. Removes ~40 lines. + +### PR 7: Migrate `scf.forall` / `scf.parallel` to ODS + +**Depends on**: PR 3. Removes ~80 lines. + +### PR 8: `function-signature(region-args($region), $function_type)` + +**Goal**: High-level directive for function-like ops. Wraps `function_interface_impl`, +handles arg attrs, result types, function type construction. + +**Depends on**: PR 1. **Scope**: ~400 lines (most complex). + +### PR 9: Migrate `func.func` to ODS assembly format + +**Depends on**: PR 8. Removes ~50 lines. + +### PR 10: Migrate `cir.func` to ODS assembly format + +**Depends on**: PR 8. Removes ~500 lines. + +--- + +### Bonus PRs: Ops That Can Move to ODS Today (No New Directives) + +ODS already handles `SingleBlockImplicitTerminator` on both parse and print sides. +The generated print code (`regionSingleBlockImplicitTerminatorPrinterCode` in +`OpFormatGen.cpp:1957`) omits the terminator when it has no attrs, no operands, +and no results — exactly matching the hand-written `omitRegionTerm` / conditional +terminator logic in custom parsers. + +These ops are custom **for no remaining reason** — they predate full ODS support: + +### PR 0a: [SCF] Migrate `scf.if` to ODS assembly format + +Already has `SingleBlockImplicitTerminator<"scf::YieldOp">` + `NoRegionArguments`. +No new traits needed. + +**ODS format:** +```tablegen +let assemblyFormat = [{ + $condition (`->` `(` type(results) `)`)? $thenRegion (`else` $elseRegion^)? attr-dict +}]; +``` + +Removes `IfOp::parse()` + `IfOp::print()` (~55 lines). +**Depends on**: nothing (can land independently before the stack). + +### PR 0b: [SCF] Migrate `scf.execute_region` to ODS assembly format + +`no_inline` is a `UnitAttr` — ODS handles `($attr^)?` natively. + +**ODS format:** +```tablegen +let assemblyFormat = [{ + (`->` type(results))? (`no_inline` $no_inline^)? $region attr-dict +}]; +``` + +Removes ~25 lines. **Depends on**: nothing. + +### PR 0c: [CIR] Add `SingleBlockImplicitTerminator` to `cir.if`, migrate to ODS + +`cir.if` currently uses hand-rolled `ensureRegionTerm`/`omitRegionTerm` helpers that +replicate what the `SingleBlockImplicitTerminator<"cir::YieldOp">` trait provides. + +**Changes:** +1. Add `SingleBlockImplicitTerminator<"cir::YieldOp">` to `CIR_IfOp` traits +2. Replace `hasCustomAssemblyFormat = 1` with: +```tablegen +let assemblyFormat = [{ + $condition $thenRegion (`else` $elseRegion^)? attr-dict +}]; +``` +3. Remove `cir::IfOp::parse()` + `cir::IfOp::print()` (~55 lines) +4. Remove `ensureRegionTerm`/`omitRegionTerm` helpers if no longer used by other ops + +**Note**: `ensureRegionTerm`/`omitRegionTerm` ARE used by other CIR ops (`cir.scope`, +global init regions). So helpers stay until those ops also migrate. + +### PR 0d: [CIR] Add `SingleBlockImplicitTerminator` to `cir.scope`, migrate to ODS + +Same pattern as `cir.if`. Currently uses `custom<OmittedTerminatorRegion>`. + +**Changes:** +1. Add `SingleBlockImplicitTerminator<"cir::YieldOp">` to `CIR_ScopeOp` traits +2. Replace format with: +```tablegen +let assemblyFormat = [{ + $scopeRegion (`:` type($results)^)? attr-dict +}]; +``` +3. Remove `parseOmittedTerminatorRegion`/`printOmittedTerminatorRegion` if unused + +--- + +### Updated PR Stack (Complete) + +``` +main + +- PR 0a: [SCF] scf.if to ODS (no deps, land first) + +- PR 0b: [SCF] scf.execute_region to ODS (no deps) + +- PR 0c: [CIR] cir.if to ODS + SingleBlockImplicitTerminator (no deps) + +- PR 0d: [CIR] cir.scope to ODS + SingleBlockImplicitTerminator (no deps) + +- PR 1: [MLIR][ODS] Core region-arg accumulation + region-arg/region-args directives + +- PR 2: [MLIR][ODS] assignment-list(region-args) directive + +- PR 3: [MLIR][ODS] region-arg type-of deferred type resolution + | +- PR 5: [SCF] Migrate scf.for to ODS assembly format + | +- PR 7: [SCF] Migrate scf.forall/scf.parallel to ODS assembly format + +- PR 4: [SCF] Migrate scf.while to ODS assembly format + +- PR 6: [Async] Migrate async.execute to ODS assembly format + +- PR 8: [MLIR][ODS] function-signature directive + +- PR 9: [Func] Migrate func.func to ODS assembly format + +- PR 10: [CIR] Migrate cir.func to ODS assembly format +``` + +PRs 0a-0d require NO new infrastructure — they use existing ODS capabilities. +They can land immediately and independently, demonstrating the pattern before +the main infrastructure work begins. + +### Estimated Scope (Updated) + +| PR | New lines | Removed lines | Complexity | +|----|-----------|--------------|------------| +| 0a | ~5 | ~55 | Trivial — format string swap | +| 0b | ~5 | ~25 | Trivial — format string swap | +| 0c | ~5 | ~55 | Trivial — add trait + format string | +| 0d | ~5 | ~30 | Trivial — add trait + format string | +| 1 | ~400 | 0 | Medium — core mechanism | +| 2 | ~200 | 0 | Low — builds on PR 1 | +| 3 | ~150 | 0 | Low — deferred type binding | +| 4 | ~15 | ~60 | Trivial — format string swap | +| 5 | ~15 | ~100 | Trivial — format string swap | +| 6 | ~10 | ~40 | Trivial — format string swap | +| 7 | ~20 | ~80 | Low — variadic version | +| 8 | ~400 | 0 | High — function_interface_impl wrapping | +| 9 | ~5 | ~50 | Trivial — format string swap | +| 10 | ~30 | ~500 | Low — format string + cleanup | + +### Files Modified (Complete) + +**Infrastructure (PRs 1-3, 8):** +- `mlir/tools/mlir-tblgen/OpFormatGen.cpp` +- `mlir/test/lib/Dialect/Test/TestOpsSyntax.td` +- `mlir/test/mlir-tblgen/op-format.mlir` + +**Quick wins (PRs 0a-0d):** +- `mlir/include/mlir/Dialect/SCF/IR/SCFOps.td` + `mlir/lib/Dialect/SCF/IR/SCF.cpp` +- `clang/include/clang/CIR/Dialect/IR/CIROps.td` + `clang/lib/CIR/Dialect/IR/CIRDialect.cpp` + +**Migrations (PRs 4-7, 9-10):** +- `mlir/include/mlir/Dialect/SCF/IR/SCFOps.td` + `mlir/lib/Dialect/SCF/IR/SCF.cpp` +- `mlir/include/mlir/Dialect/Async/IR/AsyncOps.td` + `mlir/lib/Dialect/Async/IR/Async.cpp` +- `mlir/include/mlir/Dialect/Func/IR/FuncOps.td` + `mlir/lib/Dialect/Func/IR/FuncOps.cpp` +- `clang/include/clang/CIR/Dialect/IR/CIROps.td` + `clang/lib/CIR/Dialect/IR/CIRDialect.cpp` + +**NOT modified:** +- `mlir/include/mlir/IR/OpImplementation.h` — no new OpAsmParser API +- `mlir/lib/AsmParser/Parser.cpp` — no runtime changes + +--- + +## Answers to Mehdi's Review Questions + +**"Which op in MLIR can this be applied to?"** +All 10+ ops with `hasCustomAssemblyFormat` and regions. See directive usage summary. + +**"Would this allow migrating the loop-like operations to ODS assembly format?"** +Yes. `region-arg(type-of=)` + `assignment-list(region-args)` cover all loop patterns. + +**"If you can do it with function, why wouldn't it apply to loops?"** +Same mechanism. All directives accumulate into the same `SmallVector<Argument>`. +Functions use `function-signature`, loops use `region-arg` + `assignment-list`. + +**"The generated code could construct directly the vector of region argument"** +Exactly. All directives generate code that directly builds the arg vector. No runtime +stash/take API, no state on OpAsmParser. + +**"I'm wary of ad-hoc API... explosion of APIs that don't compose"** +Six composable directives built on three existing OpAsmParser methods +(`parseArgument`, `parseArgumentList`, `parseAssignmentList`). They accumulate into +the same local vector and compose freely. _______________________________________________ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
