[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