eddyz87 added a comment.
Hi @dblaikie
Thank you for the detailed response!
> So you get some bunch of annotations from the BTFTagAttributedType, then you
> build the underlying type (which might already be built/needed/fine because
> it's used without attributes in other places/needs to exist independently) -
> and then at the end you copy any of these types that are needed and put the
> annotations on them?
Yes.
> Does the BTFTagAttributedType always have to apply to the immediate type
> that's going to be modified/mutated to have the attributes applied to it
> directly? Is the set of types that may have these attributes/annotations
> added small/closed? (struct types, void, anything else? could you add tags to
> int __tag *, for instance and get a DW_TAG_base_type for "int" with
> annotations on it?
We decided against applying it to const/volatile/restrict, but it can be
applied to struct types, void, "base" types ("int"), pointers and typedefs.
> If it's really generic/can apply to any type - but always the /immediate/
> type (I guess with the special handling you've got to skip applying it to the
> DW_TAG_const_type, etc)...
>
> What if you skipped getOrCreateType - and called into the CreateType dispatch
> below that? Since you never want a cached instance of a type, right? You want
> to create a new copy of the type you could then apply annotations to.
>
> Except, I guess, in the instance you showed, where the type is being
> completed - can't go and create another one, because we're part-way through
> building this one... hmm, maybe you can, though? What happens if we start
> making a totally distinct copy of that same type? I guess there's some map
> that contains such partially completed types, and that map would get
> confused/break if we tried to build two types for the same type without
> adding in some extra key goo that contained the annotations... - maybe that
> wouldn't be too invasive, though?
I made a prototype of such change, available here
<https://github.com/llvm/llvm-project/commit/5ad452a913cd102d38deec130a09a9e5cc2d2b30#diff-ad303855624f31f09513ee5d6e5b435714d874b6d6c2afcb140b72a4b61a9166R1257>.
The interesting function is `CGDebugInfo::CreateType(const
BTFTagAttributedType *Ty, ...)`.
Pseudo code for old version (this revision):
def CGDebugInfo::CreateType(const BTFTagAttributedType *Ty, ...):
WrappedTy, Annotations = collect annotations are base type from Ty
WrappedDI = getOrCreateType(WrappedTy, ...)
Placeholder = create temporary placeholder node with
references to WrappedDI and Annotations
AnnotationPlaceholders.push_back(Placeholder)
return Placeholder
def CGDebugInfo::finalize():
...
for Placeholder in AnnotationPlaceholders:
T = Placeholder.WrappedDI.Clone()
T.replaceAnnotations(Placeholder.Annotations)
replace Placeholder with T
Pseudo code for new version (link to my github from above):
def CGDebugInfo::CreateType(const BTFTagAttributedType *Ty, ...):
WrappedTy, Annotations = collect annotations are base type from Ty
Placeholder = empty temporary node
TypeCache[Ty] = Placeholder
WrappedDI = CreateType(UnwrapTypeForDebugInfo(WrappedTy), ...)
T = WrappedDI.Clone()
T.replaceAnnotations(Annotations)
replace Placeholder with T
return T
Comparing the "old" and "new" versions "old" appears to be less hacky to me: it
does not break `getOrCreateType()` abstraction and it does not process members
two times for circular types.
Note that this
<https://github.com/llvm/llvm-project/commit/5ad452a913cd102d38deec130a09a9e5cc2d2b30#diff-ad303855624f31f09513ee5d6e5b435714d874b6d6c2afcb140b72a4b61a9166R2805>
diff is important for the new version, without it `CreateType` won't actually
create a new object in the circular type case. I checked call-graph for
`CreateType(RecordType *)` and it looks like it is only invoked from
`getOrCreateType()`, so `getTypeOrNull()` within it is not actually needed. It
was added in commit 3b1cc9b85846 back in 2013. Test cases for that commit are
passing.
> given this code:
>
> #define __tag1 __attribute__((btf_type_tag("tag1")))
>
> struct st {
> struct st *self;
> };
> struct st __tag1 x;
>
> What's the expected DWARF here? x's type is a the attributed/annotated st
> type, but then does the self pointer inside there point to the attributed
> type or the unattributed type?
The `self` should have type w/o tag, the `x` should have type with tag.
> Maybe what you've got is the best of some bad options, but I'm not
> sure/trying to think it through. (@aprantl wouldn't mind your perspective -
> if it's too much to consume easily, let me know and I can make a best-effort
> at summarizing, maybe even better over a video chat if you've got a few
> minutes)
I appreciate the effort, if you and @aprantl decide that the call is necessary
I can attend if you need me.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D143967/new/
https://reviews.llvm.org/D143967
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
