-
Notifications
You must be signed in to change notification settings - Fork 14.9k
[CIR] Add GlobalOp ctor and dtor regions #160779
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This adds support for ctor and dtor regions in cir::GlobalOp. These regions are used to capture the code that initializes and cleans up the variable, keeping this initialization and cleanup code with the variable definition. This change only adds the CIR dialect support for these regions. Support for generating the code in these regions from source and lowering these to LLVM IR will be added in a later change, as will LoweringPrepare support to move the code into the __cxx_global_var_init() function.
@llvm/pr-subscribers-clang @llvm/pr-subscribers-clangir Author: Andy Kaylor (andykaylor) ChangesThis adds support for ctor and dtor regions in cir::GlobalOp. These regions are used to capture the code that initializes and cleans up the variable, keeping this initialization and cleanup code with the variable definition. This change only adds the CIR dialect support for these regions. Support for generating the code in these regions from source and lowering these to LLVM IR will be added in a later change, as will LoweringPrepare support to move the code into the __cxx_global_var_init() function. Full diff: https://github.com/llvm/llvm-project/pull/160779.diff 4 Files Affected:
diff --git a/clang/include/clang/CIR/Dialect/IR/CIROps.td b/clang/include/clang/CIR/Dialect/IR/CIROps.td
index bb394440bf8d8..787252e5915e0 100644
--- a/clang/include/clang/CIR/Dialect/IR/CIROps.td
+++ b/clang/include/clang/CIR/Dialect/IR/CIROps.td
@@ -683,8 +683,8 @@ def CIR_ConditionOp : CIR_Op<"condition", [
//===----------------------------------------------------------------------===//
defvar CIR_YieldableScopes = [
- "ArrayCtor", "ArrayDtor", "CaseOp", "DoWhileOp", "ForOp", "IfOp", "ScopeOp",
- "SwitchOp", "TernaryOp", "WhileOp"
+ "ArrayCtor", "ArrayDtor", "CaseOp", "DoWhileOp", "ForOp", "GlobalOp", "IfOp",
+ "ScopeOp", "SwitchOp", "TernaryOp", "WhileOp"
];
def CIR_YieldOp : CIR_Op<"yield", [
@@ -1776,7 +1776,9 @@ def CIR_GlobalLinkageKind : CIR_I32EnumAttr<
// is upstreamed.
def CIR_GlobalOp : CIR_Op<"global", [
- DeclareOpInterfaceMethods<CIRGlobalValueInterface>
+ DeclareOpInterfaceMethods<RegionBranchOpInterface>,
+ DeclareOpInterfaceMethods<CIRGlobalValueInterface>,
+ NoRegionArguments
]> {
let summary = "Declare or define a global variable";
let description = [{
@@ -1807,6 +1809,8 @@ def CIR_GlobalOp : CIR_Op<"global", [
UnitAttr:$dso_local,
OptionalAttr<I64Attr>:$alignment);
+ let regions = (region AnyRegion:$ctorRegion, AnyRegion:$dtorRegion);
+
let assemblyFormat = [{
($sym_visibility^)?
(`` $global_visibility^)?
@@ -1815,24 +1819,34 @@ def CIR_GlobalOp : CIR_Op<"global", [
(`comdat` $comdat^)?
(`dso_local` $dso_local^)?
$sym_name
- custom<GlobalOpTypeAndInitialValue>($sym_type, $initial_value)
+ custom<GlobalOpTypeAndInitialValue>($sym_type, $initial_value,
+ $ctorRegion, $dtorRegion)
attr-dict
}];
let extraClassDeclaration = [{
- bool isDeclaration() { return !getInitialValue(); }
+ bool isDeclaration() {
+ return !getInitialValue() && getCtorRegion().empty() && getDtorRegion().empty();
+ }
bool hasInitializer() { return !isDeclaration(); }
}];
let skipDefaultBuilders = 1;
- let builders = [OpBuilder<(ins
- "llvm::StringRef":$sym_name,
- "mlir::Type":$sym_type,
- CArg<"bool", "false">:$isConstant,
- // CIR defaults to external linkage.
- CArg<"cir::GlobalLinkageKind",
- "cir::GlobalLinkageKind::ExternalLinkage">:$linkage)>];
+ let builders = [
+ OpBuilder<(ins
+ "llvm::StringRef":$sym_name,
+ "mlir::Type":$sym_type,
+ CArg<"bool", "false">:$isConstant,
+ // CIR defaults to external linkage.
+ CArg<"cir::GlobalLinkageKind",
+ "cir::GlobalLinkageKind::ExternalLinkage">:$linkage,
+ CArg<"llvm::function_ref<void(mlir::OpBuilder &, mlir::Location)>",
+ "nullptr">:$ctorBuilder,
+ CArg<"llvm::function_ref<void(mlir::OpBuilder &, mlir::Location)>",
+ "nullptr">:$dtorBuilder)
+ >
+ ];
let hasVerifier = 1;
diff --git a/clang/lib/CIR/Dialect/IR/CIRDialect.cpp b/clang/lib/CIR/Dialect/IR/CIRDialect.cpp
index 58ef500446aa7..e9b8224a77ff1 100644
--- a/clang/lib/CIR/Dialect/IR/CIRDialect.cpp
+++ b/clang/lib/CIR/Dialect/IR/CIRDialect.cpp
@@ -1349,15 +1349,45 @@ mlir::LogicalResult cir::GlobalOp::verify() {
return failure();
}
+ // Verify that the constructor region, if present, has only one block which is
+ // not empty.
+ auto &ctorRegion = getCtorRegion();
+ if (!ctorRegion.empty()) {
+ if (!ctorRegion.hasOneBlock()) {
+ return emitError() << "ctor region must have exactly one block.";
+ }
+
+ auto &block = ctorRegion.front();
+ if (block.empty()) {
+ return emitError() << "ctor region shall not be empty.";
+ }
+ }
+
+ // Verify that the destructor region, if present, has only one block which is
+ // not empty.
+ auto &dtorRegion = getDtorRegion();
+ if (!dtorRegion.empty()) {
+ if (!dtorRegion.hasOneBlock()) {
+ return emitError() << "dtor region must have exactly one block.";
+ }
+
+ auto &block = dtorRegion.front();
+ if (block.empty()) {
+ return emitError() << "dtor region shall not be empty.";
+ }
+ }
+
// TODO(CIR): Many other checks for properties that haven't been upstreamed
- // yet.
+ // yet (and some that have).
return success();
}
-void cir::GlobalOp::build(OpBuilder &odsBuilder, OperationState &odsState,
- llvm::StringRef sym_name, mlir::Type sym_type,
- bool isConstant, cir::GlobalLinkageKind linkage) {
+void cir::GlobalOp::build(
+ OpBuilder &odsBuilder, OperationState &odsState, llvm::StringRef sym_name,
+ mlir::Type sym_type, bool isConstant, cir::GlobalLinkageKind linkage,
+ function_ref<void(OpBuilder &, Location)> ctorBuilder,
+ function_ref<void(OpBuilder &, Location)> dtorBuilder) {
odsState.addAttribute(getSymNameAttrName(odsState.name),
odsBuilder.getStringAttr(sym_name));
odsState.addAttribute(getSymTypeAttrName(odsState.name),
@@ -1370,26 +1400,88 @@ void cir::GlobalOp::build(OpBuilder &odsBuilder, OperationState &odsState,
cir::GlobalLinkageKindAttr::get(odsBuilder.getContext(), linkage);
odsState.addAttribute(getLinkageAttrName(odsState.name), linkageAttr);
+ Region *ctorRegion = odsState.addRegion();
+ if (ctorBuilder) {
+ odsBuilder.createBlock(ctorRegion);
+ ctorBuilder(odsBuilder, odsState.location);
+ }
+
+ Region *dtorRegion = odsState.addRegion();
+ if (dtorBuilder) {
+ odsBuilder.createBlock(dtorRegion);
+ dtorBuilder(odsBuilder, odsState.location);
+ }
+
odsState.addAttribute(getGlobalVisibilityAttrName(odsState.name),
cir::VisibilityAttr::get(odsBuilder.getContext()));
}
+/// Given the region at `index`, or the parent operation if `index` is None,
+/// return the successor regions. These are the regions that may be selected
+/// during the flow of control. `operands` is a set of optional attributes that
+/// correspond to a constant value for each operand, or null if that operand is
+/// not a constant.
+void cir::GlobalOp::getSuccessorRegions(
+ mlir::RegionBranchPoint point, SmallVectorImpl<RegionSuccessor> ®ions) {
+ // The `ctor` and `dtor` regions always branch back to the parent operation.
+ if (!point.isParent()) {
+ regions.push_back(RegionSuccessor());
+ return;
+ }
+
+ // Don't consider the ctor region if it is empty.
+ Region *ctorRegion = &this->getCtorRegion();
+ if (ctorRegion->empty())
+ ctorRegion = nullptr;
+
+ // Don't consider the dtor region if it is empty.
+ Region *dtorRegion = &this->getCtorRegion();
+ if (dtorRegion->empty())
+ dtorRegion = nullptr;
+
+ // If the condition isn't constant, both regions may be executed.
+ if (ctorRegion)
+ regions.push_back(RegionSuccessor(ctorRegion));
+ if (dtorRegion)
+ regions.push_back(RegionSuccessor(dtorRegion));
+}
+
static void printGlobalOpTypeAndInitialValue(OpAsmPrinter &p, cir::GlobalOp op,
- TypeAttr type,
- Attribute initAttr) {
+ TypeAttr type, Attribute initAttr,
+ mlir::Region &ctorRegion,
+ mlir::Region &dtorRegion) {
+ auto printType = [&]() { p << ": " << type; };
if (!op.isDeclaration()) {
p << "= ";
- // This also prints the type...
- if (initAttr)
- printConstant(p, initAttr);
+ if (!ctorRegion.empty()) {
+ p << "ctor ";
+ printType();
+ p << " ";
+ p.printRegion(ctorRegion,
+ /*printEntryBlockArgs=*/false,
+ /*printBlockTerminators=*/false);
+ } else {
+ // This also prints the type...
+ if (initAttr)
+ printConstant(p, initAttr);
+ }
+
+ if (!dtorRegion.empty()) {
+ p << " dtor ";
+ p.printRegion(dtorRegion,
+ /*printEntryBlockArgs=*/false,
+ /*printBlockTerminators=*/false);
+ }
} else {
- p << ": " << type;
+ printType();
}
}
-static ParseResult
-parseGlobalOpTypeAndInitialValue(OpAsmParser &parser, TypeAttr &typeAttr,
- Attribute &initialValueAttr) {
+static ParseResult parseGlobalOpTypeAndInitialValue(OpAsmParser &parser,
+ TypeAttr &typeAttr,
+ Attribute &initialValueAttr,
+ mlir::Region &ctorRegion,
+ mlir::Region &dtorRegion) {
mlir::Type opTy;
if (parser.parseOptionalEqual().failed()) {
// Absence of equal means a declaration, so we need to parse the type.
@@ -1397,16 +1489,50 @@ parseGlobalOpTypeAndInitialValue(OpAsmParser &parser, TypeAttr &typeAttr,
if (parser.parseColonType(opTy))
return failure();
} else {
- // Parse constant with initializer, examples:
- // cir.global @y = #cir.fp<1.250000e+00> : !cir.double
- // cir.global @rgb = #cir.const_array<[...] : !cir.array<i8 x 3>>
- if (parseConstantValue(parser, initialValueAttr).failed())
- return failure();
+ // Parse contructor, example:
+ // cir.global @rgb = ctor : type { ... }
+ if (!parser.parseOptionalKeyword("ctor")) {
+ if (parser.parseColonType(opTy))
+ return failure();
+ auto parseLoc = parser.getCurrentLocation();
+ if (parser.parseRegion(ctorRegion, /*arguments=*/{}, /*argTypes=*/{}))
+ return failure();
+ if (!ctorRegion.hasOneBlock())
+ return parser.emitError(parser.getCurrentLocation(),
+ "ctor region must have exactly one block");
+ if (ctorRegion.back().empty())
+ return parser.emitError(parser.getCurrentLocation(),
+ "ctor region shall not be empty");
+ if (ensureRegionTerm(parser, ctorRegion, parseLoc).failed())
+ return failure();
+ } else {
+ // Parse constant with initializer, examples:
+ // cir.global @y = 3.400000e+00 : f32
+ // cir.global @rgb = #cir.const_array<[...] : !cir.array<i8 x 3>>
+ if (parseConstantValue(parser, initialValueAttr).failed())
+ return failure();
- assert(mlir::isa<mlir::TypedAttr>(initialValueAttr) &&
- "Non-typed attrs shouldn't appear here.");
- auto typedAttr = mlir::cast<mlir::TypedAttr>(initialValueAttr);
- opTy = typedAttr.getType();
+ assert(mlir::isa<mlir::TypedAttr>(initialValueAttr) &&
+ "Non-typed attrs shouldn't appear here.");
+ auto typedAttr = mlir::cast<mlir::TypedAttr>(initialValueAttr);
+ opTy = typedAttr.getType();
+ }
+
+ // Parse destructor, example:
+ // dtor { ... }
+ if (!parser.parseOptionalKeyword("dtor")) {
+ auto parseLoc = parser.getCurrentLocation();
+ if (parser.parseRegion(dtorRegion, /*arguments=*/{}, /*argTypes=*/{}))
+ return failure();
+ if (!dtorRegion.hasOneBlock())
+ return parser.emitError(parser.getCurrentLocation(),
+ "dtor region must have exactly one block");
+ if (dtorRegion.back().empty())
+ return parser.emitError(parser.getCurrentLocation(),
+ "dtor region shall not be empty");
+ if (ensureRegionTerm(parser, dtorRegion, parseLoc).failed())
+ return failure();
+ }
}
typeAttr = TypeAttr::get(opTy);
diff --git a/clang/test/CIR/IR/global-init.cir b/clang/test/CIR/IR/global-init.cir
new file mode 100644
index 0000000000000..3d1389d8c3673
--- /dev/null
+++ b/clang/test/CIR/IR/global-init.cir
@@ -0,0 +1,48 @@
+// RUN: cir-opt %s -o - | FileCheck %s
+
+!u8i = !cir.int<u, 8>
+
+!rec_NeedsCtor = !cir.record<struct "NeedsCtor" padded {!u8i}>
+!rec_NeedsDtor = !cir.record<struct "NeedsDtor" padded {!u8i}>
+!rec_NeedsCtorDtor = !cir.record<struct "NeedsCtorDtor" padded {!u8i}>
+
+module attributes {cir.triple = "x86_64-unknown-linux-gnu"} {
+ cir.func private @_ZN9NeedsCtorC1Ev(!cir.ptr<!rec_NeedsCtor>)
+ cir.global external @needsCtor = ctor : !rec_NeedsCtor {
+ %0 = cir.get_global @needsCtor : !cir.ptr<!rec_NeedsCtor>
+ cir.call @_ZN9NeedsCtorC1Ev(%0) : (!cir.ptr<!rec_NeedsCtor>) -> ()
+ }
+ // CHECK: cir.global external @needsCtor = ctor : !rec_NeedsCtor {
+ // CHECK: %0 = cir.get_global @needsCtor : !cir.ptr<!rec_NeedsCtor>
+ // CHECK: cir.call @_ZN9NeedsCtorC1Ev(%0) : (!cir.ptr<!rec_NeedsCtor>) -> ()
+ // CHECK: }
+
+ cir.func private @_ZN9NeedsDtorD1Ev(!cir.ptr<!rec_NeedsDtor>)
+ cir.global external dso_local @needsDtor = #cir.zero : !rec_NeedsDtor dtor {
+ %0 = cir.get_global @needsDtor : !cir.ptr<!rec_NeedsDtor>
+ cir.call @_ZN9NeedsDtorD1Ev(%0) : (!cir.ptr<!rec_NeedsDtor>) -> ()
+ }
+ // CHECK: cir.global external dso_local @needsDtor = #cir.zero : !rec_NeedsDtor dtor {
+ // CHECK: %0 = cir.get_global @needsDtor : !cir.ptr<!rec_NeedsDtor>
+ // CHECK: cir.call @_ZN9NeedsDtorD1Ev(%0) : (!cir.ptr<!rec_NeedsDtor>) -> ()
+ // CHECK: }
+
+ cir.func private @_ZN13NeedsCtorDtorC1Ev(!cir.ptr<!rec_NeedsCtorDtor>)
+ cir.func private @_ZN13NeedsCtorDtorD1Ev(!cir.ptr<!rec_NeedsCtorDtor>)
+ cir.global external dso_local @needsCtorDtor = ctor : !rec_NeedsCtorDtor {
+ %0 = cir.get_global @needsCtorDtor : !cir.ptr<!rec_NeedsCtorDtor>
+ cir.call @_ZN13NeedsCtorDtorC1Ev(%0) : (!cir.ptr<!rec_NeedsCtorDtor>) -> ()
+ } dtor {
+ %0 = cir.get_global @needsCtorDtor : !cir.ptr<!rec_NeedsCtorDtor>
+ cir.call @_ZN13NeedsCtorDtorD1Ev(%0) : (!cir.ptr<!rec_NeedsCtorDtor>) -> ()
+ }
+ // CHECK: cir.func private @_ZN13NeedsCtorDtorC1Ev(!cir.ptr<!rec_NeedsCtorDtor>)
+ // CHECK: cir.func private @_ZN13NeedsCtorDtorD1Ev(!cir.ptr<!rec_NeedsCtorDtor>)
+ // CHECK: cir.global external dso_local @needsCtorDtor = ctor : !rec_NeedsCtorDtor {
+ // CHECK: %0 = cir.get_global @needsCtorDtor : !cir.ptr<!rec_NeedsCtorDtor>
+ // CHECK: cir.call @_ZN13NeedsCtorDtorC1Ev(%0) : (!cir.ptr<!rec_NeedsCtorDtor>) -> ()
+ // CHECK: } dtor {
+ // CHECK: %0 = cir.get_global @needsCtorDtor : !cir.ptr<!rec_NeedsCtorDtor>
+ // CHECK: cir.call @_ZN13NeedsCtorDtorD1Ev(%0) : (!cir.ptr<!rec_NeedsCtorDtor>) -> ()
+ // CHECK: }
+}
diff --git a/clang/test/CIR/IR/invalid-global.cir b/clang/test/CIR/IR/invalid-global.cir
new file mode 100644
index 0000000000000..97550588f87f6
--- /dev/null
+++ b/clang/test/CIR/IR/invalid-global.cir
@@ -0,0 +1,89 @@
+// RUN: cir-opt %s -verify-diagnostics -split-input-file
+
+!u8i = !cir.int<u, 8>
+!rec_NeedsCtor = !cir.record<struct "NeedsCtor" padded {!u8i}>
+
+module {
+ cir.global external @needsCtor = ctor : !rec_NeedsCtor {
+ }
+ // expected-error@+1 {{custom op 'cir.global' ctor region must have exactly one block}}
+}
+
+// -----
+
+!u8i = !cir.int<u, 8>
+!rec_NeedsCtor = !cir.record<struct "NeedsCtor" padded {!u8i}>
+
+module {
+ cir.func private @_ZN9NeedsCtorC1Ev(!cir.ptr<!rec_NeedsCtor>)
+ cir.global external @needsCtor = ctor : !rec_NeedsCtor {
+ %0 = cir.get_global @needsCtor : !cir.ptr<!rec_NeedsCtor>
+ cir.call @_ZN9NeedsCtorC1Ev(%0) : (!cir.ptr<!rec_NeedsCtor>) -> ()
+ cir.goto "label"
+ ^bb1:
+ cir.label "label"
+ cir.return
+ }
+ // expected-error@+1 {{custom op 'cir.global' ctor region must have exactly one block}}
+}
+
+// -----
+
+!u8i = !cir.int<u, 8>
+!rec_NeedsCtor = !cir.record<struct "NeedsCtor" padded {!u8i}>
+
+module {
+ cir.func private @_ZN9NeedsCtorC1Ev(!cir.ptr<!rec_NeedsCtor>)
+ cir.global external @needsCtor = ctor : !rec_NeedsCtor {
+ ^bb1:
+ }
+ // expected-error@+1 {{custom op 'cir.global' ctor region shall not be empty}}
+}
+
+// -----
+
+!u8i = !cir.int<u, 8>
+!rec_NeedsCtorDtor = !cir.record<struct "NeedsCtorDtor" padded {!u8i}>
+module {
+ cir.func private @_ZN13NeedsCtorDtorC1Ev(!cir.ptr<!rec_NeedsCtorDtor>)
+ cir.global external dso_local @needsCtorDtor = ctor : !rec_NeedsCtorDtor {
+ %0 = cir.get_global @needsCtorDtor : !cir.ptr<!rec_NeedsCtorDtor>
+ cir.call @_ZN13NeedsCtorDtorC1Ev(%0) : (!cir.ptr<!rec_NeedsCtorDtor>) -> ()
+ } dtor {}
+ // expected-error@+1 {{custom op 'cir.global' dtor region must have exactly one block}}
+}
+
+// -----
+
+!u8i = !cir.int<u, 8>
+!rec_NeedsCtorDtor = !cir.record<struct "NeedsCtorDtor" padded {!u8i}>
+module {
+ cir.func private @_ZN13NeedsCtorDtorC1Ev(!cir.ptr<!rec_NeedsCtorDtor>)
+ cir.global external dso_local @needsCtorDtor = ctor : !rec_NeedsCtorDtor {
+ %0 = cir.get_global @needsCtorDtor : !cir.ptr<!rec_NeedsCtorDtor>
+ cir.call @_ZN13NeedsCtorDtorC1Ev(%0) : (!cir.ptr<!rec_NeedsCtorDtor>) -> ()
+ } dtor {
+ %0 = cir.get_global @needsCtorDtor : !cir.ptr<!rec_NeedsCtorDtor>
+ cir.call @_ZN13NeedsCtorDtorD1Ev(%0) : (!cir.ptr<!rec_NeedsCtorDtor>) -> ()
+ cir.goto "label"
+ ^bb1:
+ cir.label "label"
+ cir.return
+ }
+ // expected-error@+1 {{custom op 'cir.global' dtor region must have exactly one block}}
+}
+
+// -----
+
+!u8i = !cir.int<u, 8>
+!rec_NeedsCtorDtor = !cir.record<struct "NeedsCtorDtor" padded {!u8i}>
+module {
+ cir.func private @_ZN13NeedsCtorDtorC1Ev(!cir.ptr<!rec_NeedsCtorDtor>)
+ cir.global external dso_local @needsCtorDtor = ctor : !rec_NeedsCtorDtor {
+ %0 = cir.get_global @needsCtorDtor : !cir.ptr<!rec_NeedsCtorDtor>
+ cir.call @_ZN13NeedsCtorDtorC1Ev(%0) : (!cir.ptr<!rec_NeedsCtorDtor>) -> ()
+ } dtor {
+ ^bb1:
+ }
+ // expected-error@+1 {{custom op 'cir.global' dtor region shall not be empty}}
+}
|
CArg<"cir::GlobalLinkageKind", | ||
"cir::GlobalLinkageKind::ExternalLinkage">:$linkage, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe align linkage the same way as builder below?
UnitAttr:$dso_local, | ||
OptionalAttr<I64Attr>:$alignment); | ||
|
||
let regions = (region AnyRegion:$ctorRegion, AnyRegion:$dtorRegion); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let regions = (region AnyRegion:$ctorRegion, AnyRegion:$dtorRegion); | |
let regions = (region MaxSizedRegion<1>:$ctorRegion, MaxSizedRegion<1>:$dtorRegion); |
if (!ctorRegion.hasOneBlock()) { | ||
return emitError() << "ctor region must have exactly one block."; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to check after MaxSizedRegion<1>
if (!dtorRegion.hasOneBlock()) { | ||
return emitError() << "dtor region must have exactly one block."; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to check after MaxSizedRegion<1>
clang/test/CIR/IR/global-init.cir
Outdated
@@ -0,0 +1,48 @@ | |||
// RUN: cir-opt %s -o - | FileCheck %s |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// RUN: cir-opt %s -o - | FileCheck %s | |
// RUN: cir-opt --verify-roundtrip %s -o - | FileCheck %s |
clang/test/CIR/IR/invalid-global.cir
Outdated
module { | ||
cir.global external @needsCtor = ctor : !rec_NeedsCtor { | ||
} | ||
// expected-error@+1 {{custom op 'cir.global' ctor region must have exactly one block}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This error is actually emitted by the parser. If we don't report this error, the parser will implicitly create a block with a yield statement for this case. which would be acceptable to the verifier. I'm not sure we actually need to reject this as ill-formed CIR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you are right, there's no good reason.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The same reasoning applies to the "shall not be empty" checks. So I think I can remove this entire test and the changes to GlobalOp::verify(). Because the parser implicitly creates a yield instruction, it won't be possible to test the case of an empty region.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, sounds good!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM after comment
clang/test/CIR/IR/invalid-global.cir
Outdated
module { | ||
cir.global external @needsCtor = ctor : !rec_NeedsCtor { | ||
} | ||
// expected-error@+1 {{custom op 'cir.global' ctor region must have exactly one block}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you are right, there's no good reason.
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/163/builds/27321 Here is the relevant piece of the build log for the reference
|
This adds support for ctor and dtor regions in cir::GlobalOp. These regions are used to capture the code that initializes and cleans up the variable, keeping this initialization and cleanup code with the variable definition. This change only adds the CIR dialect support for these regions. Support for generating the code in these regions from source and lowering these to LLVM IR will be added in a later change, as will LoweringPrepare support to move the code into the __cxx_global_var_init() function.
This adds support for ctor and dtor regions in cir::GlobalOp. These regions are used to capture the code that initializes and cleans up the variable, keeping this initialization and cleanup code with the variable definition.
This change only adds the CIR dialect support for these regions. Support for generating the code in these regions from source and lowering these to LLVM IR will be added in a later change, as will LoweringPrepare support to move the code into the __cxx_global_var_init() function.