[PATCH] D34618: [LLD] Add basic 64-bit SPARC support

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added a comment.

Overall looking good. Thank you for doing this!

Can you give me a pointer to the ABI documentation?



================
Comment at: lld/ELF/Arch/SPARCV9.cpp:44
+
+  PageSize = 8192;
+  DefaultMaxPageSize = 0x100000;
----------------
Is SPARC always 8k page?


================
Comment at: lld/ELF/Arch/SPARCV9.cpp:59
+  case R_SPARC_PC22:
+    if (S.getName() == "_GLOBAL_OFFSET_TABLE_")
+      return R_GOTONLY_PC;
----------------
In lld we try to avoid string comparison as it is slow. If you need to do this, you can add a new member `GlobalOffsetTable` to `ElfSym` defined in Symbols.h and compare a given symbol with that.


================
Comment at: lld/ELF/Arch/SPARCV9.cpp:61
+      return R_GOTONLY_PC;
+  // fallthrough
+  case R_SPARC_DISP32:
----------------
Use LLVM_FALLTHROUGH.


================
Comment at: lld/ELF/Arch/SPARCV9.cpp:86
+    checkUInt<32>(Loc, Val, Type);
+    write32be(Loc, Val);
+    break;
----------------
Is SPARCV9 big-endian only?


================
Comment at: lld/ELF/SyntheticSections.cpp:1093
     add({DT_PLTRELSZ, In<ELFT>::RelaPlt->getParent()->Size});
-    add({Config->EMachine == EM_MIPS ? DT_MIPS_PLTGOT : DT_PLTGOT,
-         InX::GotPlt});
+    switch (Config->EMachine) {
+    case EM_MIPS:
----------------
nit: since there are only three choices, I'd do it with if ~ else if ~ else ~.


================
Comment at: lld/ELF/SyntheticSections.cpp:1638-1639
+      HeaderSize(S) {
+  if (Config->EMachine == EM_SPARCV9)
+    this->Flags |= SHF_WRITE;
+}
----------------
This is interesting. Can you describe why the .plt segment has to be writable on Sparc? I'd like to add a comment here about the background.


https://reviews.llvm.org/D34618