Re: bpf_core_type_id_kernel is not consistent with bpf_core_type_id_local

[Yonghong Song <yonghong.song@linux.dev>]

On 11/1/23 7:16 PM, Andrii Nakryiko wrote:
> On Wed, Nov 1, 2023 at 5:34 PM Yonghong Song <yonghong.song@linux.dev> wrote:
>>
>> On 11/1/23 3:42 PM, Andrii Nakryiko wrote:
>>> On Wed, Nov 1, 2023 at 7:18 AM Lorenz Bauer <lorenz.bauer@isovalent.com> wrote:
>>>> On Tue, Oct 31, 2023 at 6:24 PM Andrii Nakryiko
>>>> <andrii.nakryiko@gmail.com> wrote:
>>>>>> Did you get round to fixing this, or did you decide to leave it as is?
>>>>> Trying to recall, was there anything to do on the libbpf side, or was
>>>>> it purely a compiler-side change?
>>>> I'm not 100% sure TBH. I'd like clang to behave consistently for
>>>> local_id and target_id. I don't know whether that would break libbpf.
>>>>
>>> *checks code* libbpf just passes through whatever ID compiler
>>> generated, so there doesn't seem to be any change to libbpf. Seems
>>> like compiler-only change. cc'ing Eduard  as well, if he's curious
>>> enough to check
>> Okay, let us try to have a consistent behavior in local/remote type_id
>> by changing local_id semantics to be the same as target_id.
>>
>> The corresponding llvm change is similar to
>>
>> [yhs@devbig309.ftw3 ~/work/llvm-project (ed)]$ git diff
>> diff --git a/llvm/lib/Target/BPF/BPFPreserveDIType.cpp b/llvm/lib/Target/BPF/BPFPreserveDIType.cpp
>> index 78e1bf90f1bd..1fbe1207dc6e 100644
>> --- a/llvm/lib/Target/BPF/BPFPreserveDIType.cpp
>> +++ b/llvm/lib/Target/BPF/BPFPreserveDIType.cpp
>> @@ -86,15 +86,17 @@ static bool BPFPreserveDITypeImpl(Function &F) {
>>          Reloc = BTF::BTF_TYPE_ID_LOCAL;
>>        } else {
>>          Reloc = BTF::BTF_TYPE_ID_REMOTE;
>> -      DIType *Ty = cast<DIType>(MD);
>> -      while (auto *DTy = dyn_cast<DIDerivedType>(Ty)) {
>> -        unsigned Tag = DTy->getTag();
>> -        if (Tag != dwarf::DW_TAG_const_type &&
>> -            Tag != dwarf::DW_TAG_volatile_type)
>> -          break;
>> -        Ty = DTy->getBaseType();
>> -      }
>> +    }
>> +    DIType *Ty = cast<DIType>(MD);
>> +    while (auto *DTy = dyn_cast<DIDerivedType>(Ty)) {
>> +      unsigned Tag = DTy->getTag();
>> +      if (Tag != dwarf::DW_TAG_const_type &&
>> +          Tag != dwarf::DW_TAG_volatile_type)
>> +        break;
>> +      Ty = DTy->getBaseType();
>> +    }
>>
>> +    if (Reloc == BTF::BTF_TYPE_ID_REMOTE) {
>>          if (Ty->getName().empty()) {
>>            if (isa<DISubroutineType>(Ty))
>>              report_fatal_error(
>> @@ -102,8 +104,8 @@ static bool BPFPreserveDITypeImpl(Function &F) {
>>            else
>>              report_fatal_error("Empty type name for BTF_TYPE_ID_REMOTE reloc");
>>          }
>> -      MD = Ty;
>>        }
>> +    MD = Ty;
>>
>>        BasicBlock *BB = Call->getParent();
>>        IntegerType *VarType = Type::getInt64Ty(BB->getContext());
>>
>> Either Eduard or Myself will submit a llvm patch to fix this in llvm18.

The change is merged into upstream llvm-project trunk ('main' branch):

https://github.com/llvm/llvm-project/commit/32e35b21b5971cc939b1de1194145d9b934fcb54


> Sounds good, and thank you!
>
>>>
>>>> Lorenz