[CIR][CodeGen] Introduce CIR CXXSpecialMember attribute #1711

bruteforceboy · 2025-06-23T13:58:37Z

I think this one is self-explanatory, so I will not write much 🙂‍

Adding this attribute helps in optimizations like #1653, and using the attribute it's easy to create operations like cir.std.vector.ctor/cir.std.vector.dtor by just modifying IdiomRecognizer a bit. I believe it will also be useful for future optimizations. Finally, I updated quite a number of tests so they now reflect this attribute.

Please, let me know if you see any issues.

bcardosolopes

Thanks for adding more of the building blocks here. In the future it'd be nice to have a representation for struct/class more closer to C++ and it will probably be easier to extract this type of information, but to prevent over engineering early, it feels right to incrementally add pieces that can enable us to better analyze C++ code and make transformations easier to write.

Can you look into changing LifetimeChecker.cpp to use this attribute instead of the current AST approach?

clang/include/clang/CIR/Dialect/IR/CIRAttrs.td

clang/test/CIR/CodeGen/String.cpp

clang/lib/CIR/CodeGen/CIRGenCall.cpp

bcardosolopes · 2025-06-23T21:53:46Z

(@andykaylor @erichkeane @dkolsen-pgi in case you have any extra thoughts here)

andykaylor

Do we also want to set this attribute on cir.func definitions/declarations?

clang/include/clang/CIR/Dialect/IR/CIRAttrs.td

clang/lib/CIR/CodeGen/CIRGenCall.cpp

clang/lib/CIR/CodeGen/CIRGenExprCXX.cpp

clang/test/CIR/CodeGen/String.cpp

github-actions · 2025-06-25T10:54:26Z

✅ With the latest revision this PR passed the C/C++ code formatter.

bruteforceboy · 2025-06-25T11:06:45Z

A few updates:

The attribute is now attached to the FuncOp (like ctor<type>) instead of the call
The attribute now stores Type instead of a string
I updated LifetimeChecker.cpp to make use of this attribute instead of the AST approach
I added a couple of traits for the ctor attribute, and I think we can extend as necessary in the future
CIR_CXXCtorDtor is now CIR_CXXSpecialMember
Updated a couple of tests to reflect the ctor/dtor attribute.

cc: @bcardosolopes, @erichkeane, @andykaylor

erichkeane

1 comment, else seems reasonable, I'm happy when the others are.

clang/lib/CIR/CodeGen/CIRGenFunction.cpp

clang/test/CIR/CodeGen/ctor-alias.cpp

clang/include/clang/CIR/Dialect/IR/CIRAttrs.td

clang/include/clang/CIR/Dialect/IR/CIROps.td

clang/include/clang/CIR/Dialect/IR/CIRAttrs.td

bcardosolopes

Thanks for the update, one more round of suggestions

clang/include/clang/CIR/Dialect/IR/CIRAttrs.td

clang/include/clang/CIR/Dialect/IR/CIROps.td

clang/lib/CIR/CodeGen/CIRGenModule.cpp

bcardosolopes

Thanks for the updates, one more round!

clang/include/clang/CIR/Dialect/IR/CIROps.td

clang/lib/CIR/Dialect/IR/CIRDialect.cpp

clang/include/clang/CIR/Dialect/IR/CIRAttrs.td

clang/lib/CIR/CodeGen/CIRGenFunction.cpp

clang/include/clang/CIR/Dialect/IR/CIRAttrs.td

clang/lib/CIR/Dialect/IR/CIRDialect.cpp

xlauko · 2025-07-07T11:50:59Z

clang/lib/CIR/Dialect/IR/CIRDialect.cpp

@@ -2798,6 +2813,15 @@ void cir::FuncOp::print(OpAsmPrinter &p) {
    p.printAttribute(annotations);
  }

+  if (getCxxSpecialMember()) {
+    p << " cxx_special_member<";


Why do we need to wrap it in cxx_special_member? It is apparent from cir.cxx_ctor/dtor that this is the special member function.

bcardosolopes

Thanks for your patience, more comments!

clang/include/clang/CIR/Dialect/IR/CIRAttrs.td

bcardosolopes · 2025-07-07T19:51:04Z

clang/lib/CIR/Dialect/IR/CIRDialect.cpp

@@ -2618,6 +2620,37 @@ ParseResult cir::FuncOp::parse(OpAsmParser &parser, OperationState &state) {
      state.addAttribute(annotationsNameAttr, annotations);
  }

+  // Parse CXXSpecialMember attribute
+  if (mlir::succeeded(parser.parseOptionalKeyword("cxx_ctor"))) {


Still sounds simple enough that we shouldn't need these to be hand written, what specific problems are you hitting?

I can update this once the syntax of the attribute is approved. I am also happy with member_function_of<type, ctor<copy>>

Thanks! I'll defer this to @xlauko cause I'm going to be out until next week. One important part here is not to have a custom parser/printer because this is simple enough that it doesn't need one (see more examples in the LLVM dialect for inspiration).

@bcardosolopes I tried working around these custom printers/parsers, and I ran into some issues.

Using the CXXCtorAttr attribute as an example, the format we want to use is member_function_of<type, ctor<Kind>>. The issue with using parseOptionalAttribute following a printAttribute, for example, to avoid manual parsing is that it tries to parse the full attribute, starting with its attribute mnemonic #cir.cxx_ctor!!

Both printAttribute and parseAttribute/parseOptionalAttribute start with the attribute mnemonic!

No matter how much you change the assembly format the mnemonic starts anyway, because CXXCtorAttr inherits from CIR_Attr. This is why I arrived at this version using the "neater parseOptionalAttribute" previously.

I think the easiest solution is having custom printers/parsers for these attributes and for CXXCtorAttr (as an example) we can print/parse the type and the ctorKind as we want, using whatever format.

I played around with these printers/parers a bit and I wasn't able to find alternatives, unfortunately.

Thanks for your patience and many different attempts here. Let's try to expedite this so you can move on to more interesting work. What I'd like to see:

No custom printing/parsing. You can drop the special_member_of and just have a #cir.cxx_ctor if that's what it takes. If you introduce an alias for it you can reduce it further, but I would leave that to another PR to land this faster.

Follow the TBAA route of attribute inheritance and drop the AnyAttrOf for the SpecialMemberOf attribute.

Thanks for the feedback.

I have updated the PR and now there is no custom printing/parsing. The constructor attribute, for example, looks like special_member<#cir.cxx_ctor<Type, Kind>>.

For AnyAttrOf, the idea was actually gotten following the TBAA route. I do not think we are able to use direct attribute inheritance. I tried and got some crashes related to attribute constraints.

clang/lib/CIR/Dialect/Transforms/LifetimeCheck.cpp

bcardosolopes · 2025-07-07T19:55:24Z

clang/test/CIR/CodeGen/temporaries.cpp

 // CIR-NEXT: cir.func private @_ZN1EntEv(!cir.ptr<!rec_E>) -> !rec_E
-// CIR-NEXT: cir.func private @_ZN1ED1Ev(!cir.ptr<!rec_E>) extra(#fn_attr)
+// CIR-NEXT: cir.func private @_ZN1ED1Ev(!cir.ptr<!rec_E>) cxx_dtor<!rec_E> extra(#fn_attr)


Sounds like we could also drop "cxx": dtor<!rec_E> is probably good enough? No need to change this right now, let's converge on the syntax first (see other comment).

xlauko · 2025-07-15T18:50:47Z

clang/include/clang/CIR/Dialect/IR/CIRAttrs.td

+def CIR_CXXCtorAttr
+    : CIR_Attr<"CXXCtor", "cxx_ctor"> {


Suggested change

def CIR_CXXCtorAttr

: CIR_Attr<"CXXCtor", "cxx_ctor"> {

def CIR_CXXCtorAttr : CIR_Attr<"CXXCtor", "cxx_ctor"> {

xlauko · 2025-07-15T18:50:57Z

clang/include/clang/CIR/Dialect/IR/CIRAttrs.td

+def CIR_CXXDtorAttr
+    : CIR_Attr<"CXXDtor", "cxx_dtor"> {


Suggested change

def CIR_CXXDtorAttr

: CIR_Attr<"CXXDtor", "cxx_dtor"> {

def CIR_CXXDtorAttr : CIR_Attr<"CXXDtor", "cxx_dtor"> {

xlauko · 2025-07-15T18:51:09Z

clang/include/clang/CIR/Dialect/IR/CIRAttrs.td

+  let assemblyFormat = [{
+    `<` $type `>`
+  }];
+  // Printing and parsing also available in CIRDialect.cpp


Suggested change

// Printing and parsing also available in CIRDialect.cpp

xlauko · 2025-07-15T18:55:47Z

clang/include/clang/CIR/Dialect/IR/CIRAttrs.td

+  let assemblyFormat = [{
+    `<` $type `,` $ctorKind `>`
+  }];
+  // Printing and parsing also available in CIRDialect.cpp


Suggested change

// Printing and parsing also available in CIRDialect.cpp

Attribute should either use default printer/parser or custom, only in latter case it is available in CIRDialect.cpp which obvious from the context.

xlauko · 2025-07-15T18:56:25Z

clang/include/clang/CIR/Dialect/IR/CIRAttrs.td

+  let builders =
+    [AttrBuilderWithInferredContext<(ins "mlir::Type":$type), [{
+      return $_get(type.getContext(), type);
+    }]>];


Suggested change

let builders =

[AttrBuilderWithInferredContext<(ins "mlir::Type":$type), [{

return $_get(type.getContext(), type);

}]>];

let builders = [

AttrBuilderWithInferredContext<(ins "mlir::Type":$type), [{

return $_get(type.getContext(), type);

}]>

];

xlauko · 2025-07-15T18:56:52Z

clang/include/clang/CIR/Dialect/IR/CIRAttrs.td

+  let builders =
+    [AttrBuilderWithInferredContext<(ins "mlir::Type":$type,
+        CArg<"CtorKind", "cir::CtorKind::Custom">:$ctorKind), [{
+      return $_get(type.getContext(), type, ctorKind);
+    }]>];


Suggested change

let builders =

[AttrBuilderWithInferredContext<(ins "mlir::Type":$type,

CArg<"CtorKind", "cir::CtorKind::Custom">:$ctorKind), [{

return $_get(type.getContext(), type, ctorKind);

}]>];

let builders = [

AttrBuilderWithInferredContext<(ins "mlir::Type":$type,

CArg<"CtorKind", "cir::CtorKind::Custom">:$ctorKind), [{

return $_get(type.getContext(), type, ctorKind);

}]>

];

bruteforceboy requested review from lanza and bcardosolopes as code owners June 23, 2025 13:58

bcardosolopes reviewed Jun 23, 2025

View reviewed changes

andykaylor reviewed Jun 24, 2025

View reviewed changes

clang/include/clang/CIR/Dialect/IR/CIRAttrs.td Outdated Show resolved Hide resolved

clang/include/clang/CIR/Dialect/IR/CIRAttrs.td Outdated Show resolved Hide resolved

erichkeane reviewed Jun 24, 2025

View reviewed changes

add CXXCtorDtor attribute

8c7235c

bruteforceboy force-pushed the cxx-ctor-dtor branch from 68ff6ef to 8c7235c Compare June 25, 2025 10:51

Merge remote-tracking branch 'origin/main' into cxx-ctor-dtor

904732f

update

d4c51f3

bruteforceboy force-pushed the cxx-ctor-dtor branch from 77c8851 to d4c51f3 Compare June 25, 2025 11:47

erichkeane reviewed Jun 25, 2025

View reviewed changes

clang/lib/CIR/CodeGen/CIRGenFunction.cpp Outdated Show resolved Hide resolved

add curleys

c3480fd

bruteforceboy changed the title ~~[CIR][CodeGen] Introduce CIR CXXCtorDtor attribute~~ [CIR][CodeGen] Introduce CIR CXXSpecialMember attribute Jun 25, 2025

bcardosolopes reviewed Jun 26, 2025

View reviewed changes

bruteforceboy added 2 commits June 27, 2025 13:10

review comments & add cir-opt

f10b62f

Merge remote-tracking branch 'origin/main' into cxx-ctor-dtor

15a4a12

bruteforceboy force-pushed the cxx-ctor-dtor branch from ab65415 to c1f8a62 Compare June 27, 2025 11:48

update test & improve parsing

f6aaa4d

bruteforceboy force-pushed the cxx-ctor-dtor branch from c1f8a62 to f6aaa4d Compare June 27, 2025 12:01

bcardosolopes reviewed Jul 1, 2025

View reviewed changes

clang/include/clang/CIR/Dialect/IR/CIRAttrs.td Outdated Show resolved Hide resolved

clang/include/clang/CIR/Dialect/IR/CIROps.td Outdated Show resolved Hide resolved

clang/lib/CIR/CodeGen/CIRGenModule.cpp Outdated Show resolved Hide resolved

bruteforceboy added 3 commits July 3, 2025 14:58

apply review comments

5de2d7a

Merge remote-tracking branch 'origin/main' into cxx-ctor-dtor

0a8fd5b

update dialect

ffe23ba

bcardosolopes reviewed Jul 3, 2025

View reviewed changes

clang/include/clang/CIR/Dialect/IR/CIROps.td Outdated Show resolved Hide resolved

clang/lib/CIR/Dialect/IR/CIRDialect.cpp Outdated Show resolved Hide resolved

bruteforceboy added 2 commits July 4, 2025 16:10

update printing/parsing

d6c0f3a

Merge remote-tracking branch 'origin/main' into cxx-ctor-dtor

db31493

xlauko reviewed Jul 7, 2025

View reviewed changes

more review comments

c40a840

bruteforceboy force-pushed the cxx-ctor-dtor branch from a5f2b05 to c40a840 Compare July 7, 2025 14:38

bcardosolopes reviewed Jul 7, 2025

View reviewed changes

xlauko reviewed Jul 15, 2025

View reviewed changes

bruteforceboy added 3 commits July 16, 2025 16:43

review comments

b7318cb

Merge remote-tracking branch 'origin/main' into cxx-ctor-dtor

470b556

refactor

5e8de9b

	def CIR_CXXCtorAttr
	: CIR_Attr<"CXXCtor", "cxx_ctor"> {
	def CIR_CXXCtorAttr : CIR_Attr<"CXXCtor", "cxx_ctor"> {

	def CIR_CXXDtorAttr
	: CIR_Attr<"CXXDtor", "cxx_dtor"> {
	def CIR_CXXDtorAttr : CIR_Attr<"CXXDtor", "cxx_dtor"> {

[CIR][CodeGen] Introduce CIR CXXSpecialMember attribute #1711

Are you sure you want to change the base?

[CIR][CodeGen] Introduce CIR CXXSpecialMember attribute #1711

Conversation

bruteforceboy commented Jun 23, 2025

Uh oh!

bcardosolopes left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bcardosolopes commented Jun 23, 2025

Uh oh!

andykaylor left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Jun 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bruteforceboy commented Jun 25, 2025

Uh oh!

erichkeane left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bcardosolopes left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bcardosolopes left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bcardosolopes left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Jun 25, 2025 •

edited

Loading