dfaust added a comment.
In D143967#4197288 <https://reviews.llvm.org/D143967#4197288>, @eddyz87 wrote:
> ...
> I think there are two sides to this:
>
> - conceptual: is it ok to allow annotations for CVR modifiers?
Maybe someone more an expert in DWARF could chime in whether this is
problematic?
As far as I know it would be "ok" in the sense that it should not break DWARF
or cause issues for DWARF consumers which do not know about the tags.
My initial reaction was that placing the annotations on CVR modifiers makes
less sense conceptually than placing them on the types proper.
But, I suppose it is a question of what precisely the annotated type is. e.g.
in `volatile int __attribute__((btf_type_tag("__b"))) b;` is it the case that:
- type tag "__b" applies to the type `volatile int` (i.e. cvr quals "bind more
closely" than the tag), or
- type tag "__b" and `volatile` both apply to `int` (i.e. cvr quals and type
tags are "equal")
For all other cases the new "btf:type_tag" format places the annotation as a
child of exactly the type that is annotated, so I think the answer determines
where the annotation logically belongs in DWARF:
- If the tag applies to `volatile int` then it should be a child of the
volatile DIE.
- If the tag applies to `int` it should be a child of the integer type DIE.
At the moment I can't say that one is more correct than the other, so I guess I
have no real objection placing the annotation on the CVR modifier.
> - technical: what is the point where such reordering is easiest to implement?
> For LLVM / pahole stack the path of least resistance is to modify DWARF
> generation (but again @dblaikie might disagree). How hard is it to adjust DI
> generation in GCC in this way?
It is not a difficult change in GCC. Some added complexity but not too much. I
have a prototype that seems to work from initial testing.
It is probably simpler than instead modifying the internal transformation to
BTF to reorder tags/CVR qualifiers as kernel currently requires.
But I don't think ease of implementation should be a factor unless we are sure
the format makes sense.
We are changing the format because the old one was flawed, let's try to make
sure we aren't just replacing it with something flawed in a different way
because it is easy.
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
