[llvm] [BOLT][AArch64] Fix LDR relocation type in ADRP+LDR sequence (PR #166391)
YongKang Zhu via llvm-commits
llvm-commits at lists.llvm.org
Tue Nov 4 10:40:37 PST 2025
yozhu wrote:
> Thanks for the update. I believe it wouldn't matter which of the 3 [RelTypes](https://github.com/llvm/llvm-project/blob/ec7bcbc6bbb237671f0ab721e88b88dcbed6fcc4/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp#L1153) we pass, as we'd get the LO12 bits expression regardless. But ofc, it's better to be accurate. You haven't observed any issues, right?
Thanks for the review and suggestions!
Right, I haven't observed any issue, before or after this change. As you said, it is just to make the relocation type name accurately reflect what kind of instruction the relocation is applied on.
> Some thoughts from the previous patch that might have helped:
> * Refactoring to reuse logic from `materializeAddress` could help
I had thought about `materializeAddress` for reuse of some of the logic (especially on creating the `ADRP` instruction), and later decided to just create a new function because the generated `ADRP+LDR` sequence is to load content from memory location and conceptually is not to materialize memory address only.
> * We could also make naming consistent between `createAdrpLdr` and `undoAdrpAddRelaxation`?
I had thought about this when working on the previous patch. `undoAdrpAddRelaxation` seems to refer to that BOLT will undo the relaxation done by the linker, but LDR (load literal) is not resulted from of any relaxation done by the linker, so I just named it `createAdrpLdr`.
https://github.com/llvm/llvm-project/pull/166391
More information about the llvm-commits
mailing list