[RISCV] Use Generated Instruction Uncompresser

[lenary]

Summary

This change allows relaxInstruction to use the auto-generated
instruction uncompression routine, rather than a partial implementation
of the same.

I am yet to find a testcase that this affects.

[lenary]

Oh there's more to do, which may involve changes to llvm/utils/TableGen/RISCVCompressInstEmitter.cpp

[lenary]

• Enable More Relaxations

[luismarques]

LGTM.

llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp

147–148

Nit: declare Res at the point of initialization or, probably better yet, just do the if on the result of uncompressInst.

[asb]

I'm getting a test failure on rv64-relax-all.s with this patch?

[lenary]

• Fix up rv64-relax-all.s testcase

[lenary]

• Address review nit.

[lenary]

• Rebase

[lenary]

Issues I still need to investigate:

• That this doesn't cause a code-size blow-up
• That this doesn't cause compiler performance regressions due to using uncompressInst in mayNeedRelaxation

However, this change is also blocking relaxing conditional branches, so I need to get to a conclusion on this quickly.

[lenary]

Preliminary data says there's no major difference to code size from this patch, so I'm feeling more confident about it. I still want to do a GCC Torture test run, if possible.

[asb]

I'm afraid this causes compile-time failures with 20030323-1.c, multi-ix.c, and pr53645.c.

e.g.

+./output_rv64imafdc_lp64_O0/multi-ix.o: in function `.LBB0_1':
+multi-ix.c:(.text+0x60): relocation truncated to fit: R_RISCV_BRANCH against `.LBB0_4'
+clang-11: error: linker command failed with exit code 1 (use -v to see invocation)

[jrtc27]

llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp

159

Only if Inst is a non-indirect branch (which is exactly the set that getRelaxedOpcode previously handled)?

llvm/test/MC/RISCV/rv64-relax-all.s

7–10

This seems like a regression?

17–21

Why not test with a non-zero immediate like before?

[lenary]

So, looking further into this for the failing test case:

• The patch as-was relaxed far more instructions than necessary in e.g. the example @asb notes is failing.
• I went back to using the explicit list of instructions that can be relaxed, from feedback from @jrtc27 - as the list is not likely to grow.
• This change is now probably pointless, as the uncompresser is *huge* and the list of instructions we should uncompress is all of 4.

llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h

 
 namespace llvm {
 class MCAssembler;
 class MCObjectTargetWriter;
 class raw_ostream;
 
 class RISCVAsmBackend : public MCAsmBackend {
   const MCSubtargetInfo &STI;
+  const MCRegisterInfo &MRI;
   uint8_t OSABI;
   bool Is64Bit;
   bool ForceRelocs = false;
   const MCTargetOptions &TargetOptions;
   RISCVABI::ABI TargetABI = RISCVABI::ABI_Unknown;
 
 public:
-  RISCVAsmBackend(const MCSubtargetInfo &STI, uint8_t OSABI, bool Is64Bit,
-                  const MCTargetOptions &Options)
-      : MCAsmBackend(support::little), STI(STI), OSABI(OSABI), Is64Bit(Is64Bit),
-        TargetOptions(Options) {
+  RISCVAsmBackend(const MCSubtargetInfo &STI, const MCRegisterInfo &MRI,
+                  uint8_t OSABI, bool Is64Bit, const MCTargetOptions &Options)
+      : MCAsmBackend(support::little), STI(STI), MRI(MRI), OSABI(OSABI),
+        Is64Bit(Is64Bit), TargetOptions(Options) {
     TargetABI = RISCVABI::computeTargetABI(
         STI.getTargetTriple(), STI.getFeatureBits(), Options.getABIName());
     RISCVFeatures::validate(STI.getTargetTriple(), STI.getFeatureBits());
   }
   ~RISCVAsmBackend() override {}
 
   void setForceRelocs() { ForceRelocs = true; }
 

llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp

 #include "llvm/MC/MCObjectWriter.h"
 #include "llvm/MC/MCSymbol.h"
 #include "llvm/MC/MCValue.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/raw_ostream.h"
 
 using namespace llvm;
 
+// Include the auto-generated portion of the uncompress emitter.
+#define GEN_UNCOMPRESS_INSTR
+#include "RISCVGenCompressInstEmitter.inc"
+
 Optional<MCFixupKind> RISCVAsmBackend::getFixupKind(StringRef Name) const {
   if (STI.getTargetTriple().isOSBinFormatELF()) {
     unsigned Type;
     Type = llvm::StringSwitch<unsigned>(Name)
 #define ELF_RELOC(X, Y) .Case(#X, Y)
 #include "llvm/BinaryFormat/ELFRelocs/RISCV.def"
 #undef ELF_RELOC
                .Default(-1u);
bool RISCVAsmBackend::fixupNeedsRelaxationAdvanced(const MCFixup &Fixup,
   case RISCV::fixup_riscv_rvc_jump:
     // For compressed jump instructions the immediate must be
     // in the range [-2048, 2046].
     return Offset > 2046 || Offset < -2048;
   }
 }
 
 void RISCVAsmBackend::relaxInstruction(MCInst &Inst,
                                        const MCSubtargetInfo &STI) const {
-  // TODO: replace this with call to auto generated uncompressinstr() function.
-  MCInst Res;
-  switch (Inst.getOpcode()) {
-  default:
-    llvm_unreachable("Opcode not expected!");
-  case RISCV::C_BEQZ:
-    // c.beqz $rs1, $imm -> beq $rs1, X0, $imm.
-    Res.setOpcode(RISCV::BEQ);
-    Res.addOperand(Inst.getOperand(0));
-    Res.addOperand(MCOperand::createReg(RISCV::X0));
-    Res.addOperand(Inst.getOperand(1));
-    break;
-  case RISCV::C_BNEZ:
-    // c.bnez $rs1, $imm -> bne $rs1, X0, $imm.
-    Res.setOpcode(RISCV::BNE);
-    Res.addOperand(Inst.getOperand(0));
-    Res.addOperand(MCOperand::createReg(RISCV::X0));
-    Res.addOperand(Inst.getOperand(1));
-    break;
-  case RISCV::C_J:
-    // c.j $imm -> jal X0, $imm.
-    Res.setOpcode(RISCV::JAL);
-    Res.addOperand(MCOperand::createReg(RISCV::X0));
-    Res.addOperand(Inst.getOperand(0));
-    break;
-  case RISCV::C_JAL:
-    // c.jal $imm -> jal X1, $imm.
-    Res.setOpcode(RISCV::JAL);
-    Res.addOperand(MCOperand::createReg(RISCV::X1));
-    Res.addOperand(Inst.getOperand(0));
-    break;
-  }
-  Inst = std::move(Res);
-}
-
-// Given a compressed control flow instruction this function returns
-// the expanded instruction.
-unsigned RISCVAsmBackend::getRelaxedOpcode(unsigned Op) const {
-  switch (Op) {
-  default:
-    return Op;
-  case RISCV::C_BEQZ:
-    return RISCV::BEQ;
-  case RISCV::C_BNEZ:
-    return RISCV::BNE;
-  case RISCV::C_J:
-  case RISCV::C_JAL: // fall through.
-    return RISCV::JAL;
-  }
+  MCInst UncompressedMI;
+  bool Res = uncompressInst(UncompressedMI, Inst, MRI, STI);
+  if (Res)
+    Inst = std::move(UncompressedMI);
 }
 
 bool RISCVAsmBackend::mayNeedRelaxation(const MCInst &Inst,
                                         const MCSubtargetInfo &STI) const {
-  return getRelaxedOpcode(Inst.getOpcode()) != Inst.getOpcode();
+  // TODO: This is inefficient, we need to generate a `isUncompressibleInst()`
+  // function in tablegen.
+  MCInst UncompressedMI;
+  return uncompressInst(UncompressedMI, Inst, MRI, STI);
 }
 
 bool RISCVAsmBackend::writeNopData(raw_ostream &OS, uint64_t Count) const {
   bool HasStdExtC = STI.getFeatureBits()[RISCV::FeatureStdExtC];
   unsigned MinNopLen = HasStdExtC ? 2 : 4;
 
   if ((Count % MinNopLen) != 0)
     return false;

 }
 
 MCAsmBackend *llvm::createRISCVAsmBackend(const Target &T,
                                           const MCSubtargetInfo &STI,
                                           const MCRegisterInfo &MRI,
                                           const MCTargetOptions &Options) {
   const Triple &TT = STI.getTargetTriple();
   uint8_t OSABI = MCELFObjectTargetWriter::getOSABI(TT.getOS());
-  return new RISCVAsmBackend(STI, OSABI, TT.isArch64Bit(), Options);
+  return new RISCVAsmBackend(STI, MRI, OSABI, TT.isArch64Bit(), Options);
 }

llvm/test/MC/RISCV/rv64-relax-all.s

 # RUN: llvm-mc -filetype=obj -triple riscv64 -mattr=+c %s | llvm-objdump -d -M no-aliases --no-show-raw-insn - | FileCheck %s --check-prefix=INSTR
 
 # RUN: llvm-mc -filetype=obj -triple riscv64 -mattr=+c %s --mc-relax-all | llvm-objdump -d -M no-aliases --no-show-raw-insn - | FileCheck %s --check-prefix=RELAX-INSTR
 
 ## Check the instructions are relaxed correctly
 
-NEAR:
+# INSTR:           c.nop
+# RELAX-INSTR:     addi    zero, zero, 0
+c.nop
 
-# INSTR:           c.beqz    a0, 0 <NEAR>
-# RELAX-INSTR:     beq    a0, zero, 0 <NEAR>
-c.beqz a0, NEAR
-
-# INSTR:           c.j    -2 <NEAR>
-# RELAX-INSTR:     jal    zero, -4 <NEAR>
-c.j NEAR
+NEAR1:
+
+# INSTR:           c.beqz    a0, 0 <NEAR1>
+# RELAX-INSTR:     beq    a0, zero, 0 <NEAR1>
+c.beqz a0, NEAR1
+
+NEAR2:
+
+# INSTR:           c.j    0 <NEAR2>
+# RELAX-INSTR:     jal    zero, 0 <NEAR2>
+c.j NEAR2