[RISCV,GISel] Unconditionally use MO_PLT for calls

[MaskRay]

All known linkers handle R_RISCV_CALL and R_RISCV_CALL_PLT in the same
way (GNU ld since
https://sourceware.org/pipermail/binutils/2020-August/112750.html).

MO_CALL is for R_RISCV_CALL, a deprecated relocation type. We don't
migrate away from MO_CALL for SelectionDAG mostly because it is an
unnecessary change. For GISel we don't have the concern and should weigh
more on simplicity.

[llvmbot]

@llvm/pr-subscribers-llvm-globalisel

@llvm/pr-subscribers-backend-risc-v

Author: Fangrui Song (MaskRay)

Changes

All known linkers handle R_RISCV_CALL and R_RISCV_CALL_PLT in the same
way (GNU ld since
https://sourceware.org/pipermail/binutils/2020-August/112750.html).

MO_CALL is for R_RISCV_CALL, a deprecated relocation type. We don't
migrate away from MO_CALL for SelectionDAG mostly because it is an
unnecessary change. For GISel we don't have the concern and should weigh
more on simplicity.

---

Full diff: https://github.com/llvm/llvm-project/pull/72355.diff

2 Files Affected:

• (modified) llvm/lib/Target/RISCV/GISel/RISCVCallLowering.cpp (+2-18)
• (modified) llvm/test/CodeGen/RISCV/GlobalISel/irtranslator/calls.ll (+2-2)

diff --git a/llvm/lib/Target/RISCV/GISel/RISCVCallLowering.cpp b/llvm/lib/Target/RISCV/GISel/RISCVCallLowering.cpp
index 37a21d5c1f2d2a0..f55421918a81323 100644
--- a/llvm/lib/Target/RISCV/GISel/RISCVCallLowering.cpp
+++ b/llvm/lib/Target/RISCV/GISel/RISCVCallLowering.cpp
@@ -496,24 +496,8 @@ bool RISCVCallLowering::lowerCall(MachineIRBuilder &MIRBuilder,
   // TODO: Support tail calls.
   Info.IsTailCall = false;
 
-  // If the callee is a GlobalAddress or ExternalSymbol and cannot be assumed as
-  // DSOLocal, then use MO_PLT. Otherwise use MO_CALL.
-  if (Info.Callee.isGlobal()) {
-    const GlobalValue *GV = Info.Callee.getGlobal();
-    unsigned OpFlags = RISCVII::MO_CALL;
-    if (!getTLI()->getTargetMachine().shouldAssumeDSOLocal(*GV->getParent(),
-                                                           GV))
-      OpFlags = RISCVII::MO_PLT;
-
-    Info.Callee.setTargetFlags(OpFlags);
-  } else if (Info.Callee.isSymbol()) {
-    unsigned OpFlags = RISCVII::MO_CALL;
-    if (!getTLI()->getTargetMachine().shouldAssumeDSOLocal(
-            *MF.getFunction().getParent(), nullptr))
-      OpFlags = RISCVII::MO_PLT;
-
-    Info.Callee.setTargetFlags(OpFlags);
-  }
+  // Select the recommended relocation type R_RISCV_CALL_PLT.
+  Info.Callee.setTargetFlags(RISCVII::MO_PLT);
 
   MachineInstrBuilder Call =
       MIRBuilder
diff --git a/llvm/test/CodeGen/RISCV/GlobalISel/irtranslator/calls.ll b/llvm/test/CodeGen/RISCV/GlobalISel/irtranslator/calls.ll
index 7b09d0c12215874..14c86a3e99e4493 100644
--- a/llvm/test/CodeGen/RISCV/GlobalISel/irtranslator/calls.ll
+++ b/llvm/test/CodeGen/RISCV/GlobalISel/irtranslator/calls.ll
@@ -427,14 +427,14 @@ define void @test_call_local() {
   ; RV32I-LABEL: name: test_call_local
   ; RV32I: bb.1.entry:
   ; RV32I-NEXT:   ADJCALLSTACKDOWN 0, 0, implicit-def $x2, implicit $x2
-  ; RV32I-NEXT:   PseudoCALL target-flags(riscv-call) @dso_local_function, csr_ilp32_lp64, implicit-def $x1
+  ; RV32I-NEXT:   PseudoCALL target-flags(riscv-plt) @dso_local_function, csr_ilp32_lp64, implicit-def $x1
   ; RV32I-NEXT:   ADJCALLSTACKUP 0, 0, implicit-def $x2, implicit $x2
   ; RV32I-NEXT:   PseudoRET
   ;
   ; RV64I-LABEL: name: test_call_local
   ; RV64I: bb.1.entry:
   ; RV64I-NEXT:   ADJCALLSTACKDOWN 0, 0, implicit-def $x2, implicit $x2
-  ; RV64I-NEXT:   PseudoCALL target-flags(riscv-call) @dso_local_function, csr_ilp32_lp64, implicit-def $x1
+  ; RV64I-NEXT:   PseudoCALL target-flags(riscv-plt) @dso_local_function, csr_ilp32_lp64, implicit-def $x1
   ; RV64I-NEXT:   ADJCALLSTACKUP 0, 0, implicit-def $x2, implicit $x2
   ; RV64I-NEXT:   PseudoRET
 entry:

[jrtc27]

I would be happy seeing SDAG changed too, and even having MO_CALL and MO_PLT merged into a single flag, with assembly always being parsed as using the PLT relocation. The pointless and historically (maybe still in GNU ld?) somewhat broken relocation needs to die.

[michaelmaitland]

LGTM. I think the SDAG equivalent and removal of RISCVII::MO_CALL can be done in a different patch, and shouldn't hold this patch up.

[topperc]

LGTM