[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

xlauko · 2025-07-07T11:26:00Z

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

+    The `default` kind is used if the constructor is a default constructor.
+    The `copy` kind is used if the constructor is a copy constructor.
+  }];
+  let parameters = (ins "mlir::Type":$type,


Why do you need type here?

If we want to tag that this is member function from specific class it might make more sense to be more generic and have attribute MemberFunctionOf that is not just tied to ctors/dtor.

Consequently these ctor/dtor attributes will be just enum attribute and unit attribute respectively.

@bcardosolopes what do you think?

Why do you need type here?

The idea is to have the type around for grabbing any extra information we might need about the class (versus making assumptions on the type of the function this/first param). We don't have a use case for it just yet tho! That's usually a somewhat blocker for me, but I'm fine with keeping this one around.

If we want to tag that this is member function from specific class it might make more sense to be more generic and have attribute MemberFunctionOf that is not just tied to ctors/dtor.

That's the idea moving forward, but starting off with ctor/dtor and go incrementally.

what do you think?

I like the idea of a member_function_of<type, ctor<copy>>.

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!

bcardosolopes · 2025-07-07T19:50:22Z

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

+    The `default` kind is used if the constructor is a default constructor.
+    The `copy` kind is used if the constructor is a copy constructor.
+  }];
+  let parameters = (ins "mlir::Type":$type,


Why do you need type here?

The idea is to have the type around for grabbing any extra information we might need about the class (versus making assumptions on the type of the function this/first param). We don't have a use case for it just yet tho! That's usually a somewhat blocker for me, but I'm fine with keeping this one around.

If we want to tag that this is member function from specific class it might make more sense to be more generic and have attribute MemberFunctionOf that is not just tied to ctors/dtor.

That's the idea moving forward, but starting off with ctor/dtor and go incrementally.

what do you think?

I like the idea of a member_function_of<type, ctor<copy>>.

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).

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).

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

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

Are you sure you want to change the base?

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

Uh oh!

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!

xlauko Jul 7, 2025

Choose a reason for hiding this comment

Uh oh!

bcardosolopes Jul 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

xlauko Jul 7, 2025

Choose a reason for hiding this comment

Uh oh!

bcardosolopes left a comment

Choose a reason for hiding this comment

Uh oh!

bcardosolopes Jul 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bcardosolopes Jul 7, 2025

Choose a reason for hiding this comment

Uh oh!

bruteforceboy Jul 9, 2025

Choose a reason for hiding this comment

Uh oh!

bcardosolopes Jul 9, 2025

Choose a reason for hiding this comment

github-actions bot commented Jun 25, 2025 •

edited

Loading

bcardosolopes Jul 7, 2025 •

edited

Loading

bcardosolopes Jul 7, 2025 •

edited

Loading