[PATCH] D56920: [PPC64] Sort .toc sections accessed with small code model relocs close to the .got

George Rimar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jan 22 02:02:34 PST 2019


grimar added a comment.

I think this is OK. Let's see what Rui think about it.
(few more nits are below)



================
Comment at: ELF/Arch/PPC64.cpp:111
+  return false;
+}
+
----------------
This would be 2 lines instead of 7 if you would use `if`.
```
  return Type == R_PPC64_GOT16 || Type == R_PPC64_TOC16 ||
         Type == R_PPC64_TOC16_DS;
```

It is common to use `if` in LLD for relocations, see: https://github.com/llvm-mirror/lld/blob/master/ELF/Arch/X86_64.cpp#L166





================
Comment at: test/ELF/ppc64-sort-small-cm-relocs.s:18
+# RUN:         .toc : {          \
+# RUN:            */ppc64-sort-small-cm-relocs.s.tmp4.o(.toc*) \
+# RUN:            */ppc64-sort-small-cm-relocs.s.tmp1.o(.toc*)  \
----------------
This relies on a test suite temporary file naming rules it seems. I am not sure if it can bring any inconsistencies or troubles, I would just use
something like

`*4.o(.toc*)`

It should also be more consistent with our other tests I think.


================
Comment at: test/ELF/ppc64-sort-small-cm-relocs.s:65
+        .quad 0xa1a1a1a1a1a1a1a1
+        .quad 0xb2b2b2b2b2b2b2b2
+
----------------
Could you remove excessive indentations in the tests, please?


Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D56920





More information about the llvm-commits mailing list