[PATCH] D17008: [lanai] Add ELF enum value and relocations
Tim Northover via llvm-commits
llvm-commits at lists.llvm.org
Fri Feb 26 07:27:46 PST 2016
t.p.northover added inline comments.
================
Comment at: include/llvm/Object/RelocVisitor.h:323
@@ +322,3 @@
+ /// Lanai ELF
+ RelocToApply visitELF_Lanai_32(RelocationRef R, uint64_t Value) {
+ int64_t Addend = getELFAddend(R);
----------------
What's the user of this function? It appears to be untested.
================
Comment at: include/llvm/Support/ELFRelocs/Lanai.def:6
@@ +5,3 @@
+
+ELF_RELOC(R_LANAI_NONE, 0)
+ELF_RELOC(R_LANAI_21, 1)
----------------
jpienaar wrote:
> eliben wrote:
> > Would this be a good place for a few lines of comments explaining each relocation type?
> Looking at the other backends, the relocation types are not normally documented in ELFRelocs/X.def. Rather the explanation of relocation types is commonly done in lib/Target/X/MCTargetDesc/XFixupKinds.h.
Fixups aren't usually in 1-1 correspondence with relocations. Normally relocs are documented in a platform's ABI document, but I Lanai's intending to do it in-tree isn't it?
If so, this file looks like a reasonable place since they're all together here.
================
Comment at: test/Object/Lanai/yaml2obj-elf-lanai-rel.yaml:6
@@ +5,3 @@
+# CHECK-NEXT: Section (2) .rel.text {
+# CHECK-NEXT: 0x0 R_LANAI_32 main 0x0
+# CHECK-NEXT: }
----------------
The other relocs should probably be tested too. It'd be good to test the actual numbers as well, if Lanai has any ABI stability expectations.
http://reviews.llvm.org/D17008
More information about the llvm-commits
mailing list