[RISCV] Support resolving fixup_riscv_call and add to MCFixupKindInfo table

[shiva0217]

Summary

And also adding static_assert after array declaration
to avoid missing any new fixup in MCFixupKindInfo in the future.

Fixup fixup_riscv_call by assembler if the function and callsite within the same compile unit and the linker relaxation disabled.

The patch is according to
https://reviews.llvm.org/D44886#inline-411881

[asb]

lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp

51

This comment should explain why we need to preserve the relocation type. e.g. "We are unable to correctly resolve a fixup_riscv_call, so always emit a relocation."

90

Thinking about it, bits should really be 64 for fixup_riscv_call. With that in place, wouldn't it be feasible to add support to adjustFixupValue so we actually can resolve the fixup?

92

You can just use array_lengthof(Infos).

94

"Not all fixup kinds added to Infos array" is probably sufficient.

[asb]

test/CodeGen/RISCV/function-call.ll

3

I think a comment explaining the purpose of the test would be useful. Presumably it's something like "Check that not attempt is made to resolve a call fixups and a relocation is always emitted". Of course this would be unnecessary if we could add support for correctly resolving call fixups!

[shiva0217]

Hi Alex.It seems that fixup fixup_riscv_call by assembler is workable. Thanks for your comments.

[asb]

lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp

278

It would be helpful to have a comment to explain the addition of 0x800ULL.

test/CodeGen/RISCV/function-call.ll

2–4

Now this test has changed to just checking the fixup is resolved when -mattr=-relax I think it makes much more sense as an MC layer test. Perhaps expanding test/MC/RISCV/fixups.s? We need some reassurance that the fixup calculation is correct, even for offsets > 12 bits.

It looks like you have to use .space as in fixups.s, as the following still results in a relocation, even though I would have expected it not to:

.equ addr, 0xdeadbeef
call addr

[asb]

test/CodeGen/RISCV/function-call.ll

2–4

Sorry - I didn't turn my brain on for that comment. It's absolutely correct that the call to an absolute target results in a relocation, as of course call is pc-relative. Any way, more tests in test/MC/RISCV/fixups.s to give reassurance that resolvable fixups are encoded correctly would be really welcome.

[shiva0217]

Update test cases to address Alex's comments.

[asb]

Thanks, looks good to me. Probably worth retitling the commit to e.g. "[RISCV] Support resolving fixup_riscv_call and add to MCFixupKindInfo table"

lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp

public:
   std::unique_ptr<MCObjectWriter>
   createObjectWriter(raw_pwrite_stream &OS) const override;
 
   // If linker relaxation is enabled, always emit relocations even if the fixup
   // can be resolved. This is necessary for correctness as offsets may change
   // during relaxation.
   bool shouldForceRelocation(const MCAssembler &Asm, const MCFixup &Fixup,
                              const MCValue &Target) override {
+    // Always preserve relocation type for function call.
+    if ((unsigned)Fixup.getKind() == RISCV::fixup_riscv_call)
+      return true;
+
     return STI.getFeatureBits()[RISCV::FeatureRelax];
   }
 
   bool fixupNeedsRelaxation(const MCFixup &Fixup, uint64_t Value,
                             const MCRelaxableFragment *DF,
                             const MCAsmLayout &Layout) const override {
     llvm_unreachable("Handled by fixupNeedsRelaxationAdvanced");
   }
 
   bool fixupNeedsRelaxationAdvanced(const MCFixup &Fixup, bool Resolved,
                                     uint64_t Value,
                                     const MCRelaxableFragment *DF,
                                     const MCAsmLayout &Layout,
                                     const bool WasForced) const override;
 
   unsigned getNumFixupKinds() const override {
     return RISCV::NumTargetFixupKinds;
   }
 
   const MCFixupKindInfo &getFixupKindInfo(MCFixupKind Kind) const override {
-    const static MCFixupKindInfo Infos[RISCV::NumTargetFixupKinds] = {
+    const static MCFixupKindInfo Infos[] = {
       // This table *must* be in the order that the fixup_* kinds are defined in
       // RISCVFixupKinds.h.
       //
       // name                      offset bits  flags
       { "fixup_riscv_hi20",         12,     20,  0 },
       { "fixup_riscv_lo12_i",       20,     12,  0 },
       { "fixup_riscv_lo12_s",        0,     32,  0 },
       { "fixup_riscv_pcrel_hi20",   12,     20,  MCFixupKindInfo::FKF_IsPCRel },
       { "fixup_riscv_pcrel_lo12_i", 20,     12,  MCFixupKindInfo::FKF_IsPCRel },
       { "fixup_riscv_pcrel_lo12_s",  0,     32,  MCFixupKindInfo::FKF_IsPCRel },
       { "fixup_riscv_jal",          12,     20,  MCFixupKindInfo::FKF_IsPCRel },
       { "fixup_riscv_branch",        0,     32,  MCFixupKindInfo::FKF_IsPCRel },
       { "fixup_riscv_rvc_jump",      2,     11,  MCFixupKindInfo::FKF_IsPCRel },
-      { "fixup_riscv_rvc_branch",    0,     16,  MCFixupKindInfo::FKF_IsPCRel }
+      { "fixup_riscv_rvc_branch",    0,     16,  MCFixupKindInfo::FKF_IsPCRel },
+      { "fixup_riscv_call",          0,     32,  MCFixupKindInfo::FKF_IsPCRel }
     };
+    static_assert((sizeof(Infos) / sizeof(MCFixupKindInfo)) ==
+                  RISCV::NumTargetFixupKinds,
+                  "the MCFixupKind numbers in Infos should equal to RISCV::NumTargetFixupKinds");
 
     if (Kind < FirstTargetFixupKind)
       return MCAsmBackend::getFixupKindInfo(Kind);
 
     assert(unsigned(Kind - FirstTargetFixupKind) < getNumFixupKinds() &&
            "Invalid kind!");
     return Infos[Kind - FirstTargetFixupKind];
   }
case RISCV::fixup_riscv_branch: {
     // Inst{31} = Sbit;
     // Inst{30-25} = Mid6;
     // Inst{11-8} = Lo4;
     // Inst{7} = Hi1;
     Value = (Sbit << 31) | (Mid6 << 25) | (Lo4 << 8) | (Hi1 << 7);
     return Value;
   }
   case RISCV::fixup_riscv_rvc_jump: {
     // Need to produce offset[11|4|9:8|10|6|7|3:1|5] from the 11-bit Value.
     unsigned Bit11  = (Value >> 11) & 0x1;
     unsigned Bit4   = (Value >> 4) & 0x1;
     unsigned Bit9_8 = (Value >> 8) & 0x3;
     unsigned Bit10  = (Value >> 10) & 0x1;
     unsigned Bit6   = (Value >> 6) & 0x1;
     unsigned Bit7   = (Value >> 7) & 0x1;
     unsigned Bit3_1 = (Value >> 1) & 0x7;
     unsigned Bit5   = (Value >> 5) & 0x1;

test/CodeGen/RISCV/function-call.ll

+; RUN: llc -mtriple=riscv32 -filetype=obj < %s\
+; RUN:     | llvm-readobj -r | FileCheck -check-prefix=RELOC %s
+
+define i32 @foo(i32 %a, i32 %b) nounwind {
+  %1 = add i32 %a, %b
+  ret i32 %1
+}
+
+define i32 @bar(i32 %a) nounwind {
+  %1 = add i32 %a, 5
+  %2 = call i32 @foo(i32 %1, i32 %a)
+; RELOC: R_RISCV_CALL foo 0x0
+  ret i32 %2
+}

test/MC/RISCV/function-call.s

 # RUN: llvm-mc -filetype=obj -triple riscv32 < %s \
 # RUN:     | llvm-objdump -d - | FileCheck -check-prefix=INSTR %s
 # RUN: llvm-mc -filetype=obj -triple riscv32 < %s \
 # RUN:     | llvm-readobj -r | FileCheck -check-prefix=RELOC %s
 # RUN: llvm-mc -triple riscv32 < %s -show-encoding \
 # RUN:     | FileCheck -check-prefix=FIXUP %s
 
 .long foo
 
 call foo
 # RELOC: R_RISCV_CALL foo 0x0
 # INSTR: auipc ra, 0
 # INSTR: jalr  ra
-# FIXUP: fixup A - offset: 0, value: foo, kind:
+# FIXUP: fixup A - offset: 0, value: foo, kind: fixup_riscv_call
 call bar
 # RELOC: R_RISCV_CALL bar 0x0
 # INSTR: auipc ra, 0
 # INSTR: jalr  ra
-# FIXUP: fixup A - offset: 0, value: bar, kind:
+# FIXUP: fixup A - offset: 0, value: bar, kind: fixup_riscv_call
 
 # Ensure that calls to functions whose names coincide with register names work.
 
 call zero
 # RELOC: R_RISCV_CALL zero 0x0
 # INSTR: auipc ra, 0
 # INSTR: jalr  ra
-# FIXUP: fixup A - offset: 0, value: zero, kind:
+# FIXUP: fixup A - offset: 0, value: zero, kind: fixup_riscv_call
 
 call f1
 # RELOC: R_RISCV_CALL f1 0x0
 # INSTR: auipc ra, 0
 # INSTR: jalr  ra
-# FIXUP: fixup A - offset: 0, value: f1, kind:
+# FIXUP: fixup A - offset: 0, value: f1, kind: fixup_riscv_call
 
 call ra
 # RELOC: R_RISCV_CALL ra 0x0
 # INSTR: auipc ra, 0
 # INSTR: jalr  ra
-# FIXUP: fixup A - offset: 0, value: ra, kind:
+# FIXUP: fixup A - offset: 0, value: ra, kind: fixup_riscv_call