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