Outliner separates ADRP and LDR into separate sections

[pranavk]

This came up in #131630 where we are trying to fix the a case where outliner separates ADRP and LDR instructions into two separate sections which leads to problems that #131630 is trying to fix.

@smithp35 mentioned that this shouldn't happen in the first place because it prevents an optimization that converts GOT indirection to PC-relative addressing.

Unfortunately, I don't have a minimal repro I can share as of now but this happens with our internal code where outliner exhibits this.

[llvmbot]

@llvm/issue-subscribers-lld-elf

Author: Pranav Kant (pranavk)

This came up in #131630 where we are trying to fix the a case where outliner separates ADRP and LDR instructions into two separate sections which leads to problems that #131630 is trying to fix.

@smithp35 mentioned that this shouldn't happen in the first place because it prevents an optimization that converts GOT indirection to PC-relative addressing.

Unfortunately, I don't have a minimal repro I can share as of now but this happens with our internal code where outliner exhibits this.

[llvmbot]

@llvm/issue-subscribers-backend-aarch64

Author: Pranav Kant (pranavk)

This came up in #131630 where we are trying to fix the a case where outliner separates ADRP and LDR instructions into two separate sections which leads to problems that #131630 is trying to fix.

@smithp35 mentioned that this shouldn't happen in the first place because it prevents an optimization that converts GOT indirection to PC-relative addressing.

Unfortunately, I don't have a minimal repro I can share as of now but this happens with our internal code where outliner exhibits this.

[lenary]

I'm going to provide some RISC-V context here, though I know this is an AArch64 issue.

In RISC-V, we specifically have to disable machine outlining where it would split pc-relative high/low relocations into separate sections. The code to do so is here:

llvm-project/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp

Lines 3165 to 3168 in b9d27ac

	if (MO.getTargetFlags() == RISCVII::MO_PCREL_LO &&
	(MI.getMF()->getTarget().getFunctionSections() || F.hasComdat() ||
	F.hasSection() || F.getSectionPrefix()))
	return outliner::InstrType::Illegal;

and has been continually added to as we found new sources of "this might split the outlined body into a separate section from the original functions". In particular, the RISC-V ABI requires that the pc-relative high/low relocations are in the same section.

I think where RISC-V differs from AArch64 in this case is that on AArch64, the low 12 bits of the materialized address are not preserved by ADRP, whereas they are in RISC-V's auipc (i.e., there could be a carry when applying the low relocation). I think this has the effect of meaning that on AArch64, splitting the two parts of the pair apart is less dangerous than on RISC-V, even if on AArch64 it prevents some linker optimisations.

I note the Machine Outliner was written by apple developers who also worked on the AArch64 backend. AArch64 Mach-O does have linker optimisation hints as well, but of course not quite to the degree that RISC-V's ELF psABI has them in linker relaxations.

[smithp35]

I think it is safe for the outliner to make the transformation as the LDR and ADD are not PC-relative. Splitting the ADRP and its LDR/ADD into separate sections may impede binary analysis, such as being able to turn a GOT load into a PC relative one. I guess this is a trade off the outliner will need to make of saving code-size over performance.