yonghong-song marked 4 inline comments as done and an inline comment as not 
done.
yonghong-song added inline comments.


================
Comment at: llvm/lib/Target/BPF/BPFCORE.h:17
+  enum OffsetRelocKind : uint32_t {
+    FIELD_ACCESS_OFFSET = 0,
+    FIELD_EXISTENCE,
----------------
ast wrote:
> why ACCESS_OFFSET is necessary?
> Isn't it the same as BYTE_OFFSET but for non-bitfield?
> May be single kind will do?
This is the relocation type in the .BTF.ext. FIELD_ACCESS_OFFSET corresponds to 
odd offset relocation. The below newly added relocations are for field info.
I am using relocation enum except FIELD_ACCESS_OFFSET to be used for field_info 
in __builtin_preserve_field_info().


================
Comment at: llvm/lib/Target/BPF/BPFCORE.h:22
+    FIELD_BYTE_OFFSET,
+    FIELD_LSHIFT_64BIT_BUF,
+    FIELD_RSHIFT_AFTER_LSHIFT_64BIT_BUF,
----------------
ast wrote:
> All these names are not added as builtin enum before compilation starts, 
> right?
> So C program cannot just use FIELD_BYTE_OFFSET and needs to define its own 
> enum first?
> How about FIELD_LSHIFT_U64 instead?
Yes, C programs should define its own enum to have corresponding values first.
Do not familiar within builtin enum's. Let me take a look.

FIELD_LSHIFT_U64 is shorter and looks good.


================
Comment at: llvm/lib/Target/BPF/BPFCORE.h:23
+    FIELD_LSHIFT_64BIT_BUF,
+    FIELD_RSHIFT_AFTER_LSHIFT_64BIT_BUF,
+
----------------
ast wrote:
> FIELD_RSHIFT_U64 ?
Yes, sounds better.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67980



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

Reply via email to