[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_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.


More information about the llvm-commits mailing list