[PATCH] D81457: [LLD][PowerPC] Add support for R_PPC64_PCREL34

[Sean Fertile via Phabricator via llvm-commits]

sfertile added a comment.

Do you want to add a test for overflow of a 34-bit value?



================
Comment at: lld/ELF/Arch/PPC64.cpp:381
+// endianness). So on little endian machines, we have to load/store the
+// pieces separately and assemble/disassemble them accordingly.
+static void writePrefixedInstruction(uint8_t *loc, uint64_t insn) {
----------------
This comments a out of date (ie load/store the pieces separately).  If you mention its  a 4 byte prefix followed by a 4 byte instruction I think the shifting we have to do for LE becomes pretty obvious.


================
Comment at: lld/test/ELF/ppc64-reloc-pcrel34.s:37
+	.long	0
+	.size	glob_int8, 8
+
----------------
Do you want to define an int of size 4 or 8? you use an lwa so I would expect a 4 byte int, and you use .long which allocates 4 bytes (I belive its the same as .int), but then give it a size of 8 and name it as int8? Ditto for `glob_int8_big`


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

https://reviews.llvm.org/D81457