[RISCV] Add R_RISCV_RELAX relocation to all possible relax candidates.

[kito-cheng]

Summary

Add R_RISCV_RELAX relocation to all possible relax candidates and corresponding testcase.

[kito-cheng]

Changes:

• Rebase.
• Change fixup value of R_RISCV_RELAX to a constant 0 value for consistent with binutils's implementation.

[asb]

lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp

198

It would make most sense to declare and initialise this alongside Expr/Kind/FixupKind, seeing as like those RelaxCandidate is relevant only for immediates.

266–267

Suggest a comment here, something like:
"Ensure an R_RISCV_RELAX relocation will be emitted if linker relaxation is enabled and the current fixup will result in a relocation that may be relaxed."

test/MC/RISCV/linker-relaxation.s

27

Check you add a check that R_RISCV_RELAX is not generated when -mattr=-relax is used?

[kito-cheng]

Changes:

• Move declaration of RelaxCandidate.
• Add comment.
• Update testcase

[asb]

I think we've landed enough pre-requisites that this this is good to go in, but it introduces failures to option-pushpop.s and option-relax.s. Can you take a look please?

[kito-cheng]

Okay, I'll take a look :)

[kito-cheng]

Changes:

• Update testcase, R_RISCV_RELAX won't bind with symbol to fit binutils's behavior.

[asb]

Looks good to me, thanks.

lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp

 unsigned RISCVMCCodeEmitter::getImmOpValue(const MCInst &MI, unsigned OpNo,
                                            SmallVectorImpl<MCFixup> &Fixups,
                                            const MCSubtargetInfo &STI) const {
   bool EnableRelax = STI.getFeatureBits()[RISCV::FeatureRelax];
   const MCOperand &MO = MI.getOperand(OpNo);
 
   MCInstrDesc const &Desc = MCII.get(MI.getOpcode());
   unsigned MIFrm = Desc.TSFlags & RISCVII::InstFormatMask;
 
   // If the destination is an immediate, there is nothing to do.
   if (MO.isImm())
     return MO.getImm();
 
   assert(MO.isExpr() &&
          "getImmOpValue expects only expressions or immediates");
   const MCExpr *Expr = MO.getExpr();
   MCExpr::ExprKind Kind = Expr->getKind();
   RISCV::Fixups FixupKind = RISCV::fixup_riscv_invalid;
+  bool RelaxCandidate = false;
   if (Kind == MCExpr::Target) {
     const RISCVMCExpr *RVExpr = cast<RISCVMCExpr>(Expr);
 
     switch (RVExpr->getKind()) {
     case RISCVMCExpr::VK_RISCV_None:
     case RISCVMCExpr::VK_RISCV_Invalid:
       llvm_unreachable("Unhandled fixup kind!");
     case RISCVMCExpr::VK_RISCV_LO:
       if (MIFrm == RISCVII::InstFormatI)
         FixupKind = RISCV::fixup_riscv_lo12_i;
       else if (MIFrm == RISCVII::InstFormatS)
         FixupKind = RISCV::fixup_riscv_lo12_s;
       else
         llvm_unreachable("VK_RISCV_LO used with unexpected instruction format");
+      RelaxCandidate = true;
       break;
     case RISCVMCExpr::VK_RISCV_HI:
       FixupKind = RISCV::fixup_riscv_hi20;
+      RelaxCandidate = true;
       break;
     case RISCVMCExpr::VK_RISCV_PCREL_LO:
       if (MIFrm == RISCVII::InstFormatI)
         FixupKind = RISCV::fixup_riscv_pcrel_lo12_i;
       else if (MIFrm == RISCVII::InstFormatS)
         FixupKind = RISCV::fixup_riscv_pcrel_lo12_s;
       else
         llvm_unreachable(
             "VK_RISCV_PCREL_LO used with unexpected instruction format");
+      RelaxCandidate = true;
       break;
     case RISCVMCExpr::VK_RISCV_PCREL_HI:
       FixupKind = RISCV::fixup_riscv_pcrel_hi20;
+      RelaxCandidate = true;
       break;
     case RISCVMCExpr::VK_RISCV_CALL:
       FixupKind = RISCV::fixup_riscv_call;
+      RelaxCandidate = true;
       break;
     }
   } else if (Kind == MCExpr::SymbolRef &&
              cast<MCSymbolRefExpr>(Expr)->getKind() == MCSymbolRefExpr::VK_None) {
     if (Desc.getOpcode() == RISCV::JAL) {
       FixupKind = RISCV::fixup_riscv_jal;
     } else if (MIFrm == RISCVII::InstFormatB) {
       FixupKind = RISCV::fixup_riscv_branch;
     } else if (MIFrm == RISCVII::InstFormatCJ) {
       FixupKind = RISCV::fixup_riscv_rvc_jump;
     } else if (MIFrm == RISCVII::InstFormatCB) {
       FixupKind = RISCV::fixup_riscv_rvc_branch;
     }
   }
 
   assert(FixupKind != RISCV::fixup_riscv_invalid && "Unhandled expression!");
 
   Fixups.push_back(
       MCFixup::create(0, Expr, MCFixupKind(FixupKind), MI.getLoc()));
   ++MCNumFixups;
 
-  if (EnableRelax) {
-    if (FixupKind == RISCV::fixup_riscv_call) {
-      Fixups.push_back(
-      MCFixup::create(0, Expr, MCFixupKind(RISCV::fixup_riscv_relax),
-                      MI.getLoc()));
-      ++MCNumFixups;
-    }
-  }
+  // Ensure an R_RISCV_RELAX relocation will be emitted if linker relaxation is
+  // enabled and the current fixup will result in a relocation that may be
+  // relaxed.
+  if (EnableRelax && RelaxCandidate) {
+    const MCConstantExpr *Dummy = MCConstantExpr::create(0, Ctx);
+    MCFixup::create(0, Dummy, MCFixupKind(RISCV::fixup_riscv_relax),
 
   return 0;
 }
 
 #include "RISCVGenMCCodeEmitter.inc"

test/MC/RISCV/linker-relaxation.s

 
 .long foo
 
 .L1:
 call foo
 # NORELAX-RELOC: R_RISCV_CALL foo 0x0
 # NORELAX-RELOC-NOT: R_RISCV_RELAX
 # RELAX-RELOC: R_RISCV_CALL foo 0x0
-# RELAX-RELOC: R_RISCV_RELAX foo 0x0
+# RELAX-RELOC: R_RISCV_RELAX - 0x0
 # RELAX-FIXUP: fixup A - offset: 0, value: foo, kind: fixup_riscv_call
-# RELAX-FIXUP: fixup B - offset: 0, value: foo, kind: fixup_riscv_relax
+# RELAX-FIXUP: fixup B - offset: 0, value: 0, kind: fixup_riscv_relax
 beq s1, s1, .L1
 # RELAX-RELOC: R_RISCV_BRANCH .L1 0x0
 # RELAX-FIXUP: fixup A - offset: 0, value: .L1, kind: fixup_riscv_branch
+# RELAX-FIXUP-NOT: fixup B - offset: 0, value: 0, kind: fixup_riscv_relax
+
+lui t1, %hi(foo)
+# NORELAX-RELOC: R_RISCV_HI20 foo 0x0
+# NORELAX-RELOC-NOT: R_RISCV_RELAX
+# RELOC: R_RISCV_HI20 foo 0x0
+# RELAX-RELOC: R_RISCV_RELAX - 0x0
+# RELAX-FIXUP: fixup A - offset: 0, value: %hi(foo), kind: fixup_riscv_hi20
+# RELAX-FIXUP: fixup B - offset: 0, value: 0, kind: fixup_riscv_relax
+
+addi t1, t1, %lo(foo)
+# NORELAX-RELOC: R_RISCV_LO12_I foo 0x0
+# NORELAX-RELOC-NOT: R_RISCV_RELAX
+# RELOC: R_RISCV_LO12_I foo 0x0
+# RELAX-RELOC: R_RISCV_RELAX - 0x0
+# RELAX-FIXUP: fixup A - offset: 0, value: %lo(foo), kind: fixup_riscv_lo12_i
+# RELAX-FIXUP: fixup B - offset: 0, value: 0, kind: fixup_riscv_relax
+
+sb t1, %lo(foo)(a2)
+# NORELAX-RELOC: R_RISCV_LO12_S foo 0x0
+# NORELAX-RELOC-NOT: R_RISCV_RELAX
+# RELOC: R_RISCV_LO12_S foo 0x0
+# RELAX-RELOC: R_RISCV_RELAX - 0x0
+# RELAX-FIXUP: fixup A - offset: 0, value: %lo(foo), kind: fixup_riscv_lo12_s
+# RELAX-FIXUP: fixup B - offset: 0, value: 0, kind: fixup_riscv_relax
+
+auipc t1, %pcrel_hi(foo)
+# NORELAX-RELOC: R_RISCV_PCREL_HI20 foo 0x0
+# NORELAX-RELOC-NOT: R_RISCV_RELAX
+# RELOC: R_RISCV_PCREL_HI20 foo 0x0
+# RELAX-RELOC: R_RISCV_RELAX - 0x0
+# RELAX-FIXUP: fixup A - offset: 0, value: %pcrel_hi(foo), kind: fixup_riscv_pcrel_hi20
+# RELAX-FIXUP: fixup B - offset: 0, value: 0, kind: fixup_riscv_relax
+
+addi t1, t1, %pcrel_lo(foo)
+# NORELAX-RELOC: R_RISCV_PCREL_LO12_I foo 0x0
+# NORELAX-RELOC-NOT: R_RISCV_RELAX
+# RELOC: R_RISCV_PCREL_LO12_I foo 0x0
+# RELAX-RELOC: R_RISCV_RELAX - 0x0
+# RELAX-FIXUP: fixup A - offset: 0, value: %pcrel_lo(foo), kind: fixup_riscv_pcrel_lo12_i
+# RELAX-FIXUP: fixup B - offset: 0, value: 0, kind: fixup_riscv_relax
+
+sb t1, %pcrel_lo(foo)(a2)
+# NORELAX-RELOC: R_RISCV_PCREL_LO12_S foo 0x0
+# NORELAX-RELOC-NOT: R_RISCV_RELAX
+# RELOC: R_RISCV_PCREL_LO12_S foo 0x0
+# RELAX-RELOC: R_RISCV_RELAX - 0x0
+# RELAX-FIXUP: fixup A - offset: 0, value: %pcrel_lo(foo), kind: fixup_riscv_pcrel_lo12_s
+# RELAX-FIXUP: fixup B - offset: 0, value: 0, kind: fixup_riscv_relax