================
@@ -502,6 +502,15 @@ def CIR_RecordType : CIR_Type<"Record", "record",
void complete(llvm::ArrayRef<mlir::Type> members, bool packed,
bool isPadded);
+
+ // Utilities for lazily computing and cacheing data layout info.
+ // FIXME: currently opaque because there's a cycle if CIRTypes.types include
+ // from CIRAttrs.h. The implementation operates in terms of RecordLayoutAttr
+ // instead.
+ private:
+ mutable mlir::Attribute layoutInfo;
----------------
bcardosolopes wrote:
Some extra context: we use an attribute here as an implementation detail /
convenience, `RecordLayoutAttr` was never intended to be parsed/printed, all we
need here is cache functionality. We do want such cache to be tied to the IR
(as you pointed out) because different passes might be able to reuse/explore
the information.
> If the goal was to have it in the IR, I would expect some kay-value storage
> similar to DLTI_MapAttr and store it at modul level?
That's probably more pristine MLIR usage, feels it still keeps the intent I
mentioned above.
> it won't retain any of the layoutInfo, as it will be default constructed.
This is not a problem in practice: it will lazily computed again if what you
get via `dyn_cast` precisely because the value is default constructed (and
therefore empty/invalid).
> I would suggest to have just extra methods to get specific layouInfo
> parameterers in such case as they are recomputed at every RecordType
> construction anyway.
Honestly, with all the FUD / negativity regarding compile time we already have
towards CIR, I'd love if we don't make the story worse just because we're
trying to be "MLIR purists". My suggestion is to keep the approach as-is for
this PR and as follow up either (a) prove somehow this isn't impacting compile
time and change the impl to be eager or (b) change `RecordLayoutAttr` to use
`DLTI_MapAttr` as you suggested. The same change should be done in the
incubator to allow us to try evaluating the impact.
https://github.com/llvm/llvm-project/pull/136036
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
