[CIR][Transforms] Introduce StdVectorCtorOp & StdVectorDtorOp

[bruteforceboy]

In this PR, I continue work on Issue#1653.

This PR introduces std.vector_cxx_ctor and std.vector_cxx_dtor building on the special member attributes from PR#1711. Extending the implementation for other std constructors/destructors is quite easy too. For example, to introduce std::map, the only changes needed are:

+++ b/clang/include/clang/CIR/Dialect/IR/CIRStdOps.td
+def CIR_StdMapCtorOp: CIR_StdOp<"map_cxx_ctor",
+  (ins CIR_AnyType:$first),
+  (outs Optional<CIR_AnyType>:$result)>;

+++ b/clang/lib/CIR/Dialect/Transforms/IdiomRecognizer.cpp
@@ -239,7 +239,7 @@ void IdiomRecognizerPass::recognizeCall(CallOp call) {
   using StdFunctionsRecognizer =
       std::tuple<StdRecognizer<StdFindOp>, StdRecognizer<StdVectorCtorOp>,
-                 StdRecognizer<StdVectorDtorOp>>;
+                 StdRecognizer<StdVectorDtorOp>, StdRecognizer<StdMapCtorOp>>;

To get this to work, I updated the CXXCtorAttr and CXXDtorAttr special member attributes to also have the source clang::RecordDecl. A few things that may be worth discussing are:

• Do we get rid of the mlir::Type in both attributes now that we have clang::RecordDecl?
• Should we also print/parse the clang::RecordDecl? For now, I think it's unnecessary, because it is pretty much the same information as the mlir::Type.

I have also added one test to demo the added operations. Please, let me know your thoughts. Thanks!

[bcardosolopes]

While here, can we also add statistics counts (https://mlir.llvm.org/docs/PassManagement/#pass-statistics) to these? It's going to be super useful for finding opportunities soon. It's fine it comes in a next PR.

[bcardosolopes]

vector_cxx_ctor -> vector.ctor

[bcardosolopes]

Similar for dtor!

[bcardosolopes]

These remarks are great, thanks! I think we need to adjust the names a bit given std::vector_cxx_ctor() doesn't really exist. Perhaps found call to std::vector constructor and similar for dtor?

[bcardosolopes]

It's also possible if in the future we want to generalize ctor/dtors in a different way, but I think this is a good start to get more data on how these things are going to be useful.

(cc @tommymcm)

[bcardosolopes]

• Do we get rid of the mlir::Type in both attributes now that we have clang::RecordDecl?

How hard would be to use mlir::Type to get the same information this PR gets from clang::RecordDecl? Would we need to augment cir.record much? We probably want to keep the record type lightweight, but it's a tradeoff.

[bcardosolopes]

• Should we also print/parse the clang::RecordDecl? For now, I think it's unnecessary, because it is pretty much the same information as the mlir::Type.

This is harder than it seems, because for that we'd want to use AST serialization/deserialization (and be able to roundtrip tests, etc) - this is a known limitation of keeping the AST around. As you noticed, it forces us into writing tests from source instead of CIR to CIR.

[bcardosolopes]

(cc @andykaylor)

[tommymcm]

• Do we get rid of the mlir::Type in both attributes now that we have clang::RecordDecl?

How hard would be to use mlir::Type to get the same information this PR gets from clang::RecordDecl? Would we need to augment cir.record much? We probably want to keep the record type lightweight, but it's a tradeoff.

Would it be better to just introduce a cir.std.vector type at this point? This would also let us clean up duplicated ops: instead of having a ctor/dtor op for each std type we care about, we could have a single cir.ctor with a type specifier , so cir.std.vector.ctor would become cir.ctor ... : cir.ptr<cir.std.vector<...>>.

I am interested in helping out with this if folks think its a good direction.

(I don't think that this is necessary for this PR, I agree with Bruno that having something working with statistics will help us know what needs to be changed/refined)

[andykaylor]

I realize I'm jumping in to the middle of the design/implement cycle, but I have a series of nested questions about why this isn't more general.

Why do we need a vector-specific CIR_StdVectorCtorOp rather than a general CIR_StdCtrOp that takes the class type as an argument/attribute. Which leads me to, why does this need to be limited to std rather than just a general CIR_CallCtorOp with an attribute telling you what it's constructing in a way that the constructee could easily be recognized as a standard libary class. And then, why do we need a dedicated op at all, as opposed to just attaching the cxx_ctor attribute to the callsite?

The two primary advantages I see for the latter are (1) you wouldn't need any extra handling for call vs. invoke, and (2) any transformation that was trying to operate on calls would find these structors without needing special handling.

[andykaylor]

I'd like to see examples here in some form. A form that would put the example in the generated HTML pages for the dialect would be best, but at a minimum, some comment here would be helpful, especially if the tests aren't going to show the full CIR generated.