[PATCH] D97986: BPF: permit type modifiers for __builtin_btf_type_id() relocation

[Andrii Nakryiko via Phabricator via llvm-commits]

anakryiko added inline comments.


================
Comment at: llvm/lib/Target/BPF/BPFPreserveDIType.cpp:91
+        if (Tag != dwarf::DW_TAG_const_type &&
+            Tag != dwarf::DW_TAG_volatile_type)
+          break;
----------------
yonghong-song wrote:
> anakryiko wrote:
> > should we strip `restrict` as well?
> I deliberately skipped the `restrict`. in C, `restrict` is used to restrict pointers and used in pointer declarations (include variables and parameters). 
>   https://en.wikipedia.org/wiki/Restrict
> In type casting, the `restrict` will be ignored. For example, 
> ```
> -bash-4.4$ cat tt.c
> struct s {
>   int a;
> };
> int test() {
>   return __builtin_btf_type_id(*(struct s * restrict)0, 1);
> }
> -bash-4.4$ clang -target bpf -O2 -S -g tt.c -emit-llvm -Xclang -disable-llvm-passes
> ```
> restrict does not show up in the debuginfo.
> 
> I tried to construct another example,
> ```
> -bash-4.4$ cat tt.c                                                               
> struct s {
>   int a;
> };
> 
> int test() {
>   struct s * restrict v;
>   return __builtin_btf_type_id(*v, 1);
> }
> ```
> In this case, "restrict" shows up in variable declaration as restrict -> pointer -> struct s.
> The meta data we attached is the pointee type "struct s", so we are fine.
> 
> typically `restrict` shows up before pointer like [restrict|const|volatile]+ pointer [const|volatile]+ non-pointer type.
> 
> What I did is trying to remove const/volatile in "[const|volatile]+ non-pointer-type". So we should be fine here.
> 
> 
ok, thanks for checking, Yonghong!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97986