[PATCH] D108280: [JITLink] Optimize GOTPCRELX Relocations
Lang Hames via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Aug 18 02:19:40 PDT 2021
lhames accepted this revision.
lhames added a comment.
This revision is now accepted and ready to land.
This is very cool -- thanks Stephen!
Please address the inline comments, otherwise LGTM!
================
Comment at: llvm/lib/ExecutionEngine/JITLink/x86_64.cpp:82-84
+ auto *BlockData = reinterpret_cast<uint8_t *>(
+ const_cast<char *>(B->getContent().data())) +
+ E.getOffset();
----------------
`BlockData` should be `FixupData`.
I know that was my name originally, but now that this function is getting more attention we should fix my old mistakes. ;)
================
Comment at: llvm/lib/ExecutionEngine/JITLink/x86_64.cpp:95
JITTargetAddress TargetAddr = GOTTarget.getAddress();
-
+ JITTargetAddress EdgeAddr = B->getAddress() + E.getOffset();
int64_t Displacement = TargetAddr - EdgeAddr + 4;
----------------
The preferred way to express this is `JITTargetAddress EdgeAddr = B->getFixupAddress(E)`.
================
Comment at: llvm/lib/ExecutionEngine/JITLink/x86_64.cpp:100-101
+
+ if (!(TargetInRangeForImmU32 || DisplacementInRangeForImmS32))
+ continue;
+
----------------
I assume the rationale here is "if neither of these is true then there's no point looking at the instruction sequence more closely"? I would add a comment for that to make it clear.
================
Comment at: llvm/test/ExecutionEngine/JITLink/X86/ELF_x86-64_small_pic_relocations.s:71-81
# Test GOTPCREL handling. We want to check both the offset to the GOT entry and its
# contents.
# jitlink-check: decode_operand(test_gotpcrel, 4) = \
# jitlink-check: got_addr(elf_sm_pic_reloc.o, named_data) - next_pc(test_gotpcrel)
# jitlink-check: *{8}(got_addr(elf_sm_pic_reloc.o, named_data)) = named_data
.globl test_gotpcrel
----------------
Could you add to the comment to note that `leal` is used here to suppress optimization (in case anybody looks at it and wonders why it isn't a mov).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D108280/new/
https://reviews.llvm.org/D108280
More information about the llvm-commits
mailing list