dblaikie added a comment.
In D74572#2042931 <https://reviews.llvm.org/D74572#2042931>, @yonghong-song
wrote:
> @dblaikie I can revert but let me first understand the alternative way to do
> the work, at least in high level.
>
> [
> I do believe my commit message, esp. the first couple of lines is badly
> inaccurate. I suspect this is the one causing your confusion as well.
> specially, In the beginning of commit message, I said:
>
> u32 btf_type_id = __builtin_btf_type_id(param)
>
> But later on, I also said:
>
> In the above example, the second argument for __builtin_btf_type_id() is 0,
> which means a relocation for local adjustment, i.e., w.r.t. bpf program BTF
> change, will be generated. The value 1 for the second argument means a
> relocation for remote adjustment, e.g., against vmlinux.
>
>
> The above statement implies two arguments.
> The actually builtin signature should be
>
> u32 btf_type_id = __builtin_btf_type_id(expr, reloc_type)
>
> The first parameter is not necessary a parameter and it can be any expression.
> This is my fault as this patch go through several revision and I forgot to
> change that. I am happy to revert and rework the commit message to make it
> more clear.
> ]
Appreciate the clarification - thanks!
> Maybe, the description not clear or too BPF specific. Let me try again to
> describe the purpose of this builtin.
> The interface of this builtin likes below:
>
> __builtin_btf_type_id(expr, reloc_type)
>
> The goal is to:
>
> - the expression here can be a variable (like "a" int "struct { ...} a", or
> any valid C expression, e.g., &a, a->field_b).
> - return the type_id, and generates a reloc_type based on user input (the
> relocation is generated in .BTF.ext section).
> - here, the relocation is against the return value, e.g., unsigned btf_id =
> __builtin_btf_type_id(expr, reloc_type) a relocation against the BPF
> instruction bpf_id = ... needs to be generated.
>
> What is the use case for this builtin? Currently, various uses are all for
> pretty-print. Pass some data together with btf_id so kernel or use space can
> pretty-print it. For example struct {void *p, uint32_t bpf_id} v = {a->b->c,
> __builtin_btf_type_id(a->b->c, 1)}; printk("....%pT...", ..., &v, ...); here
> a->b->c might be a type "struct abc", and the kernel will be able to print
> out the object pointed by a->b->c in a pretty format based on "struct abc".
> Here, the second parameter "1" means the ressulted btf_id will be relocated
> against kernel BTF. The linux kernel target is x64, arm64, etc.
>
> In the future, we may have other use cases.
>
> The reason we need relocation is later BTF debug info may be merged
> (linked) with other bpf program and you need to adjust btf_id, or it may try
> to match against types against vmlinux.
>
> The type associated with the expression may be local which user has control
> or maybe in a header user does not have control.
>
> The following is what I understand the possible alternative:
> - Keep or the retained type for the expression, so we do not need the IR
> builtin. I guess, in order to do this, we need:
> - We still need to annotate the expression somehow so we later on we can
> generate proper relocation later on.
> - If we do have a variable here, we might be able to annotate the variable
> (special attributes?) so we do not carry debuginfo types. But for
> expressions, we would still need to carry debuginfo. Otherwise, it is very
> hard to get debug info in backend.
>
> Hopefully this is more clear with the interface/intention of this builtin.
> Please let me know in high level whether we could do better. Thanks!
Thanks for the explanation (& sorry for my very delayed response).
BPF uses its own debug info format, right? I guess it might be far enough out
of my wheelhouse that I don't have a lot of understanding/experience to impart
into your situation there. I think the explanation makes some sense to me as to
why you're using an intrinsic like this, and I was a bit quick to call for a
revert/suggest this wasn't the right direction - sorry about that.
It might be worth having an extra test that's just IR->IR to test the
BPFPreserveDIType pass in a more isolated fashion.
Or would it be reasonable to have Clang generate the IR in the
post-BPFPreserveDIType form to begin with, and avoid the need for that pass to
run later?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D74572/new/
https://reviews.llvm.org/D74572
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
