yonghong-song added inline comments.
================
Comment at: clang/include/clang/Basic/Attr.td:1835
+ let Args = [StringArgument<"BTFTag">];
+ let Subjects = SubjectList<[Var, Function, Record, Field], ErrorDiag>;
+ let Documentation = [BTFTagDocs];
----------------
aaron.ballman wrote:
> ObjC method declarations?
>
> Also, can this apply to *any* kind of variable declaration? e.g., does it do
> something useful on a `constexpr` variable that never gets emitted at runtime?
The attribute named btf_tag and it is supposed to be used for bpf programs and
kernel, and currently all our use cases are C, so hence ObjC is not considered
and I marked it as COnly.
constexpr is not our use case so it is okay the variable (and possible
attricute) is gone.
================
Comment at: clang/include/clang/Basic/AttrDocs.td:2019
+targets. This attribute may be attached to a struct/union, struct/union field,
+function or variables declaration. If -g is specified, the ARGUMENT info will
+be preserved in IR and be emitted to dwarf. For BPF target, the ARGUMENT info
----------------
aaron.ballman wrote:
> yonghong-song wrote:
> > anakryiko wrote:
> > > can it be also applied to function's argument?
> > The attribute does apply to function argument. I missed it in the
> > documentation. Will add it.
>
thanks! Will change.
================
Comment at: clang/include/clang/Basic/AttrDocs.td:2020
+function or variables declaration. If -g is specified, the ARGUMENT info will
+be preserved in IR and be emitted to dwarf. For BPF target, the ARGUMENT info
+will be emitted to .BTF ELF section too.
----------------
aaron.ballman wrote:
>
Thanks. Will change.
================
Comment at: clang/test/Sema/attr-btf_tag.c:15
+ return arg->a;
+}
----------------
aaron.ballman wrote:
> There are quite a few test cases that are missing. Can you please add tests
> for applying the attribute to something invalid (like an enumeration), not
> given any arguments, given too many arguments.
>
> Also missing are test cases for how this attribute merges on redeclarations,
> especially with conflicting arguments. e.g.,
> ```
> void __attribute__((btf_tag("tag"))) bar();
> void __attribute__((btf_tag("derp"))) bar() {} // Which argument "wins" or
> should this be an error?
> ```
> Should it be valid to apply this to an incomplete structure declaration? e.g.,
> ```
> struct __attribute__((btf_tag("tag"))) S; // Should this be valid or invalid?
> ```
Thanks. I will add more test cases about redeclaration. For redeclaration case,
my current thinking is they should match *exactly*. Otherwise, we should fail
the compilation. I guess some implementation may be needed here.
For forward declaration, the current intention is not allowed. the btf_tag
should just appear in the actual type definition.
For other types like enum, we didn't find a use case for that yet and that is
why is not supported. I will add more tests to cover such invalid cases.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D106614/new/
https://reviews.llvm.org/D106614
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
