[PATCH] D101336: [LLD][BPF] Add bpf support

[Yonghong Song via Phabricator via llvm-commits]

yonghong-song added a comment.

For this `Why is this <unknown>?` if you remove `if (config->emachine != EM_BPF && type != target->noneRel)`, this is because I didn't implement BPF target relocate properly. In current llvm BPF backend, BPFAsmBackend.cpp file, we have

  void BPFAsmBackend::applyFixup(const MCAssembler &Asm, const MCFixup &Fixup,
                                 const MCValue &Target,
                                 MutableArrayRef<char> Data, uint64_t Value,
                                 bool IsResolved,
                                 const MCSubtargetInfo *STI) const {
    if (Fixup.getKind() == FK_SecRel_4 || Fixup.getKind() == FK_SecRel_8) {
      // The Value is 0 for global variables, and the in-section offset
      // for static variables. Write to the immediate field of the inst.
      assert(Value <= UINT32_MAX);
      support::endian::write<uint32_t>(&Data[Fixup.getOffset() + 4],
                                       static_cast<uint32_t>(Value),
                                       Endian);
    } else if (Fixup.getKind() == FK_Data_4) {
      support::endian::write<uint32_t>(&Data[Fixup.getOffset()], Value, Endian);
    } else if (Fixup.getKind() == FK_Data_8) {
      support::endian::write<uint64_t>(&Data[Fixup.getOffset()], Value, Endian);
    } else if (Fixup.getKind() == FK_PCRel_4) {
      Value = (uint32_t)((Value - 8) / 8);
      if (Endian == support::little) {
        Data[Fixup.getOffset() + 1] = 0x10;
        support::endian::write32le(&Data[Fixup.getOffset() + 4], Value);
      } else {
        Data[Fixup.getOffset() + 1] = 0x1;
        support::endian::write32be(&Data[Fixup.getOffset() + 4], Value);
      } 
    } else {
      assert(Fixup.getKind() == FK_PCRel_2);
      Value = (uint16_t)((Value - 8) / 8);
      support::endian::write<uint16_t>(&Data[Fixup.getOffset() + 2], Value,
                                       Endian);
    }                                  
  }

You can see some relocation data is written at the instruction offset 2 or 4. In my current relocate() function, the start of the insn (which include insn opcode) is written instead of offset 4. That is why you will see "<unknown>" here.

For

>> I would suggest that you try figuring out how other REL tests work (by commenting out some !RelTy::IsRela code and seeing what tests would fail).

That is a good suggestion, I am certainly not a ldd expert. I will look at the related codes, remove the above hack and resubmit the patch again.



================
Comment at: lld/ELF/Arch/BPF.cpp:56
+  default:
+    return R_ABS;
+  }
----------------
MaskRay wrote:
> See other getRelExpr implementations. The allowed relocation types must be listed.
will do.


================
Comment at: lld/test/ELF/lto/bpf-diff-sec.ll:1
+; REQUIRES: bpf
+; RUN: llvm-as %S/Inputs/bpf-diff-sec-1.ll -o %t1.bc
----------------
MaskRay wrote:
> The auxiliary file is only used once. Please use split-file
will do.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101336