[PATCH] D44886: [RISCV] Support linker relax function call from auipc and jalr to jal

[Alex Bradbury via Phabricator via llvm-commits]

asb added inline comments.


================
Comment at: lib/Target/RISCV/MCTargetDesc/RISCVFixupKinds.h:50
   fixup_riscv_rvc_branch,
+  // fixup_riscv_call - For generate R_RISCV_CALL to relax function call.
+  fixup_riscv_call,
----------------
The description indicates you'd only want to use the call fixup when relaxation is enabled. But there's nothing stopping us from using fixup_riscv_call even when linker relaxation is disabled - it reduces the number of relocations for the linker to process.

How about something like the following, which better explains its use "A fixup representing a call attached to the auipc instruction of an adjacent auipc+pair."


================
Comment at: lib/Target/RISCV/MCTargetDesc/RISCVFixupKinds.h:52
+  fixup_riscv_call,
+  // fixup_riscv_relax - For generate R_RISCV_RELAX relax relocation type.
+  fixup_riscv_relax,
----------------
Maybe something like "Used to generate an R_RISCV_RELAX relocation type, which indicates the linker may relax the instruction pair."


================
Comment at: test/CodeGen/RISCV/linker-relaxation.ll:8
+
+define i32 @test_call_external(i32 %a) nounwind {
+; RELAX-RELOC: R_RISCV_CALL external_function 0x0
----------------
I think we need to test the assembly output here, which will show that %pcrel_lo/%pcrel_hi are still emitted (and also checks that the asm emitter code doesn't crash when encountering VK_RISCV_CALL_LO). It's obviously unfortunate that generating .s and then assembling that will produce a different result (two relocations) than emitting the .o directly. Emitting the 'call` pseudoinstruction as used by gcc would fix that issue, but still leaves you with no way of expressing the "expanded" auipc+jalr form.


Repository:
  rL LLVM

https://reviews.llvm.org/D44886