[CIR][CIRGen][TBAA] Add support for scalar types

[PikachuHyA]

No description provided.

[PikachuHyA]

follow #1076 (review)

This patch adds support for TBAA with scalar types. It retrieves type names in the TBAAPass, utilizing as much information from the CIR as possible, rather than duplicating the original codegen implementation in CIRGenTBAA.

A limitation of this patch is that it cannot handle long long types. Instead, long long is treated as long. Specifically, long long is represented as s64i in CIR, and I have mapped s64i to the long type.

[PikachuHyA]

I have update the patch. please review again.

• add NYI if no tests covered
• add tests (tbaa-struct.cpp and tbaa-vtpr.cpp)
• format tablegen file

A limitation of this patch is that it cannot handle long long types. Instead, long long is treated as long. Specifically, long long is represented as s64i in CIR, and I have mapped s64i to the long type.

I would appreciate your thoughts on this limitation.

[bcardosolopes]

Our internal builds use -O2 to build our apps, and seems like this path and some of the others will always run under -O2. For paths that will unconditionally run, you are better off with, e.g. assert(!MissingFeature::tbaaIncompleteType()) or something. Can you confirm that all added unrecheables are never executed just yet? An alternative is to add a temporary cc1 flag and only enable tbaa when the flag is used, once you are done with the full work you would remove the flag.

[PikachuHyA]

Our internal builds use -O2 to build our apps, and seems like this path and some of the others will always run under -O2

did not encounter the NYI issue when run with -O2. it may be related to the struct type.

I think adding MissingFeatures::tbaaIncompleteType() would be a good idea.

[bcardosolopes]

I would appreciate your thoughts on this limitation.

Haven't had the time to dig just yet, we can look at that once this is closer to be merged!

[PikachuHyA]

Changes Summary

• rename CIR_TBAAScalarTypeDescriptorAttr to CIR_TBAAScalarAttr
• change the type of tbaa parameter from ArrayAttr to CIR_AnyTBAAAttr
• remove TBAAPass and lowering CIR_TBAAScalarAttr in ConvertCIRToLLVMPass
• add MissingFeatures::tbaaVTablePtr and MissingFeatures::tbaaIncompleteType

[PikachuHyA]

I noticed the LowerXXX classes (e.g., LowerModule, LowerTypes) in clang/lib/CIR/Dialect/Transforms/TargetLowering.

How about adding a LowerTBAA class to encapsulate TBAA lowering-related functions (e.g., getChar, lowerScalarType) and state management (e.g., some struct cache)?

[bcardosolopes]

Just a few more nits and this looks ready!

[bcardosolopes]

I noticed the LowerXXX classes (e.g., LowerModule, LowerTypes) in clang/lib/CIR/Dialect/Transforms/TargetLowering.

If structs change as part of ABI lowering or new ones get created, we'll need to update information accordingly, but I think those can be handled directly as part of LowerTypes and whatnots.

How about adding a LowerTBAA class to encapsulate TBAA lowering-related functions (e.g., getChar, lowerScalarType)

LowerXXX is CIR to CIR. It seems to me that until LLVM lowering, we don't need any of those, since they are LLVM specific.

and state management (e.g., some struct cache)?

Attributes are value semantic, so MLIR should cache things for us. Do you have an example of something that might escape that?

[PikachuHyA]

Attributes are value semantic, so MLIR should cache things for us. Do you have an example of something that might escape that?

Let's discuss the scenario where I have submitted patch support for struct types.
For scalar types, we can disregard it.

[bcardosolopes]

LGTM, thanks for your patience!

For long long, can you create an issue describing the problem and we can discuss it there? My feeling is that it will need some type changes in CIR to accommodate and will require it's own PR anyways.

[PikachuHyA]

For long long, can you create an issue describing the problem and we can discuss it there? My feeling is that it will need some type changes in CIR to accommodate and will require it's own PR anyways.

added #1241