https://github.com/adams381 updated https://github.com/llvm/llvm-project/pull/214074
>From c8b8b555164ed5f08042ff207c2e2e9102e33ed7 Mon Sep 17 00:00:00 2001 From: Adam Smith <[email protected]> Date: Tue, 4 Aug 2026 13:56:58 -0700 Subject: [PATCH 1/2] [CIR] Verify record_align is a non-zero power of two `#cir.record_layout` carries `record_align`, which CIRGen fills from `ASTRecordLayout::getAlignment()` and consumers read as an `llvm::Align`. That constructor asserts the value is a non-zero power of two, so hand-written CIR naming any other alignment aborted the tool rather than reporting a parse error. A zero tripped the non-zero assert and a 3 tripped the power-of-two one, both inside `llvm::Align` with no indication of which attribute was at fault. Verify the field where it is parsed. Values CIRGen emits are already well-formed, so this only affects hand-written input. Assisted-by: Cursor / claude-opus-5 --- .../include/clang/CIR/Dialect/IR/CIRAttrs.td | 2 ++ clang/lib/CIR/Dialect/IR/CIRAttrs.cpp | 16 ++++++++++++++++ clang/test/CIR/IR/invalid-record-layout.cir | 19 +++++++++++++++++++ 3 files changed, 37 insertions(+) create mode 100644 clang/test/CIR/IR/invalid-record-layout.cir diff --git a/clang/include/clang/CIR/Dialect/IR/CIRAttrs.td b/clang/include/clang/CIR/Dialect/IR/CIRAttrs.td index a0fe997156a69..5042ec9ab05aa 100644 --- a/clang/include/clang/CIR/Dialect/IR/CIRAttrs.td +++ b/clang/include/clang/CIR/Dialect/IR/CIRAttrs.td @@ -169,6 +169,8 @@ def CIR_RecordLayoutAttr : CIR_Attr<"RecordLayout", "record_layout"> { `>` }]; + let genVerifyDecl = 1; + let canHaveIllegalCXXABIType = 0; } diff --git a/clang/lib/CIR/Dialect/IR/CIRAttrs.cpp b/clang/lib/CIR/Dialect/IR/CIRAttrs.cpp index 264e836718c81..48cedbb44a856 100644 --- a/clang/lib/CIR/Dialect/IR/CIRAttrs.cpp +++ b/clang/lib/CIR/Dialect/IR/CIRAttrs.cpp @@ -920,6 +920,22 @@ LogicalResult DynamicCastInfoAttr::verify( // RecordLayout lookup //===----------------------------------------------------------------------===// +LogicalResult +RecordLayoutAttr::verify(function_ref<InFlightDiagnostic()> emitError, + cir::ArgPassingKind argPassingKind, + bool hasTrivialDtor, uint64_t recordAlign) { + // record_align comes from ASTRecordLayout::getAlignment() and is consumed as + // an llvm::Align, which requires a non-zero power of two. Reject anything + // else here so hand-written CIR gets a diagnostic instead of an assertion + // failure inside whichever pass reads the field. + if (recordAlign == 0) + return emitError() << "record_align must be non-zero"; + if (!llvm::isPowerOf2_64(recordAlign)) + return emitError() << "record_align must be a power of two, got " + << recordAlign; + return success(); +} + RecordLayoutAttr cir::getRecordLayout(mlir::ModuleOp module, mlir::StringAttr name) { auto dict = module->getAttrOfType<mlir::DictionaryAttr>( diff --git a/clang/test/CIR/IR/invalid-record-layout.cir b/clang/test/CIR/IR/invalid-record-layout.cir new file mode 100644 index 0000000000000..fa54ef6977acf --- /dev/null +++ b/clang/test/CIR/IR/invalid-record-layout.cir @@ -0,0 +1,19 @@ +// RUN: cir-opt %s -verify-diagnostics -split-input-file + +module attributes { + cir.record_layouts = { + // expected-error @below {{record_align must be non-zero}} + S = #cir.record_layout<arg_passing_kind = can_pass_in_regs, + has_trivial_dtor = true, record_align = 0>} +} { +} + +// ----- + +module attributes { + cir.record_layouts = { + // expected-error @below {{record_align must be a power of two, got 3}} + S = #cir.record_layout<arg_passing_kind = can_pass_in_regs, + has_trivial_dtor = true, record_align = 3>} +} { +} >From 75abbfc6961306a93cd67d9f5fae286319361f1b Mon Sep 17 00:00:00 2001 From: Adam Smith <[email protected]> Date: Tue, 4 Aug 2026 18:30:19 -0700 Subject: [PATCH 2/2] [CIR] Verify record_align with a PredAttrTrait Replace the hand-written verifier with the predicate ODS already emits into `verifyInvariantsImpl`. The check only ever fires on hand-written CIR, so naming the offending value did not justify a custom verifier to maintain. The diagnostic becomes a single fixed string. Assisted-by: Cursor / claude-opus-5 --- clang/include/clang/CIR/Dialect/IR/CIRAttrs.td | 10 +++++++--- clang/lib/CIR/Dialect/IR/CIRAttrs.cpp | 16 ---------------- clang/test/CIR/IR/invalid-record-layout.cir | 4 ++-- 3 files changed, 9 insertions(+), 21 deletions(-) diff --git a/clang/include/clang/CIR/Dialect/IR/CIRAttrs.td b/clang/include/clang/CIR/Dialect/IR/CIRAttrs.td index 5042ec9ab05aa..71585cd83fb66 100644 --- a/clang/include/clang/CIR/Dialect/IR/CIRAttrs.td +++ b/clang/include/clang/CIR/Dialect/IR/CIRAttrs.td @@ -123,7 +123,13 @@ def CIR_ArgPassingKind : CIR_I32EnumAttr< let genSpecializedAttr = 0; } -def CIR_RecordLayoutAttr : CIR_Attr<"RecordLayout", "record_layout"> { +def CIR_RecordLayoutAttr : CIR_Attr<"RecordLayout", "record_layout", [ + // record_align is consumed as an llvm::Align, whose constructor asserts a + // non-zero power of two. + PredAttrTrait<"record_align must be a non-zero power of two", + CPred<"$record_align != 0 && " + "::llvm::isPowerOf2_64($record_align)">> + ]> { let summary = "ABI layout metadata for a record type"; let description = [{ Holds AST-derived ABI metadata for a named record type. These @@ -169,8 +175,6 @@ def CIR_RecordLayoutAttr : CIR_Attr<"RecordLayout", "record_layout"> { `>` }]; - let genVerifyDecl = 1; - let canHaveIllegalCXXABIType = 0; } diff --git a/clang/lib/CIR/Dialect/IR/CIRAttrs.cpp b/clang/lib/CIR/Dialect/IR/CIRAttrs.cpp index 48cedbb44a856..264e836718c81 100644 --- a/clang/lib/CIR/Dialect/IR/CIRAttrs.cpp +++ b/clang/lib/CIR/Dialect/IR/CIRAttrs.cpp @@ -920,22 +920,6 @@ LogicalResult DynamicCastInfoAttr::verify( // RecordLayout lookup //===----------------------------------------------------------------------===// -LogicalResult -RecordLayoutAttr::verify(function_ref<InFlightDiagnostic()> emitError, - cir::ArgPassingKind argPassingKind, - bool hasTrivialDtor, uint64_t recordAlign) { - // record_align comes from ASTRecordLayout::getAlignment() and is consumed as - // an llvm::Align, which requires a non-zero power of two. Reject anything - // else here so hand-written CIR gets a diagnostic instead of an assertion - // failure inside whichever pass reads the field. - if (recordAlign == 0) - return emitError() << "record_align must be non-zero"; - if (!llvm::isPowerOf2_64(recordAlign)) - return emitError() << "record_align must be a power of two, got " - << recordAlign; - return success(); -} - RecordLayoutAttr cir::getRecordLayout(mlir::ModuleOp module, mlir::StringAttr name) { auto dict = module->getAttrOfType<mlir::DictionaryAttr>( diff --git a/clang/test/CIR/IR/invalid-record-layout.cir b/clang/test/CIR/IR/invalid-record-layout.cir index fa54ef6977acf..61ebc9b37528c 100644 --- a/clang/test/CIR/IR/invalid-record-layout.cir +++ b/clang/test/CIR/IR/invalid-record-layout.cir @@ -2,7 +2,7 @@ module attributes { cir.record_layouts = { - // expected-error @below {{record_align must be non-zero}} + // expected-error @below {{failed to verify that record_align must be a non-zero power of two}} S = #cir.record_layout<arg_passing_kind = can_pass_in_regs, has_trivial_dtor = true, record_align = 0>} } { @@ -12,7 +12,7 @@ module attributes { module attributes { cir.record_layouts = { - // expected-error @below {{record_align must be a power of two, got 3}} + // expected-error @below {{failed to verify that record_align must be a non-zero power of two}} S = #cir.record_layout<arg_passing_kind = can_pass_in_regs, has_trivial_dtor = true, record_align = 3>} } { _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
