yonghong-song added a comment.

In D83242#2193989 <https://reviews.llvm.org/D83242#2193989>, @anakryiko wrote:

>> This is different than testing whether a type exists or not where we want to 
>> precisely test whether that type exists or not, so we want to preserve 
>> `typedef` type vs. `struct t` type.
>
> `typedef t` and `struct t` are also different for field relocations (those t 
> is in different namespaces, actually), so it might be a good idea to 
> disambiguate those for field relocations as well?

Just to be clear. The type recorded in relocation will be always valid since we 
have type_id there. For field exists, due to we may use nullptr as the object, 
if CSE happens, it is possible the root type id may be the "struct t" instead 
of "typedef t" (or the other ordering) assuming the definition "typedef struct 
t t;". But from root type_id, libbpf will be able to correctly trace down to 
the field and do the relocation properly.

On the other hand, I agree that this is not ideal as we may use a different 
root type than what user specified in the source. But everything is hidden and 
invisible to user so I won't have user visible impact.

To fix this in clang is actually tricky, we do not want to add seq number to 
every preserve_field_info* IR builtin since this is not really necessary for 
non-existence relocations. Also preserve_field_info* although used by bpf only 
so far is llvm generic builtin, I would hesitate to change it with new seq_num 
fields. The best will be have another bpf specific builtin
for field existence, but given the current mechanism also works, I probably 
will keep it this way.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D83242/new/

https://reviews.llvm.org/D83242

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to