This is an archive of the discontinued LLVM Phabricator instance.

[flang][NFC] Wrap TBAABuilder in unique_ptr
AbandonedPublic

Authored by springerm on Aug 10 2023, 6:13 AM.

Details

Summary

The fir::LLVMTypeConverter has a TBAABuilder. The current design prevents the type converter stored in conversion patterns from being const. This is generally dangerous because type conversion rules should not change during a dialect conversion.

This revision wraps the TBAABuilder in a unique_ptr. That way, the type converter itself can be const, but the TBAABuilder can still have mutable state. (What's important form an MLIR perspective is that the type conversion rules cannot change.)

This change is needed for D157601.

Diff Detail

Event Timeline

springerm created this revision.Aug 10 2023, 6:13 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald Transcript
springerm requested review of this revision.Aug 10 2023, 6:13 AM
springerm retitled this revision from [flang] Move TBAABuilder to ConversionPattern to [flang] Move TBAABuilder to FIROpConversion.Aug 10 2023, 6:23 AM
springerm added inline comments.
flang/include/flang/Optimizer/CodeGen/TypeConverter.h
149

Alternatively, this could be turned into a TBAABuilder * const.

There is the following comment in TypeConverter.cpp:LLVMTypeConverter::convertBoxType:

// TODO: send the box type and the converted LLVM structure layout
// to tbaaBuilder for proper creation of TBAATypeDescriptorOp.

In future, we may need the original and the converted box type to create more precise TBAA type descriptor. I am not sure if it will be convenient to do outside of the type converter. If you insist on making this change, please, make sure you remove the TODO comment.

keep TBAABuilder in type converter

springerm retitled this revision from [flang] Move TBAABuilder to FIROpConversion to [flang] Wrap TBAABuilder in unique_ptr.Aug 11 2023, 12:38 AM
springerm edited the summary of this revision. (Show Details)

There is the following comment in TypeConverter.cpp:LLVMTypeConverter::convertBoxType:

// TODO: send the box type and the converted LLVM structure layout
// to tbaaBuilder for proper creation of TBAATypeDescriptorOp.

In future, we may need the original and the converted box type to create more precise TBAA type descriptor. I am not sure if it will be convenient to do outside of the type converter. If you insist on making this change, please, make sure you remove the TODO comment.

OK, then let's keep the TBAABuilder there for the moment. Wrapping it in a unique_ptr also works. I just want to make sure that the base type conversion state (i.e., conversion rules) is immutable.

springerm retitled this revision from [flang] Wrap TBAABuilder in unique_ptr to [flang][NFC] Wrap TBAABuilder in unique_ptr.Aug 11 2023, 12:44 AM
springerm abandoned this revision.Aug 14 2023, 12:04 AM

merged into D157601