[RISCV 9/10] Add support for disassembly

[asb]

Summary

Disassembler support allows for 'round-trip' testing, and rv32i-valid.s has been updated appropriately.

[reames]

lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp

44 ↗

(On Diff #68193)

Style: Should these be in a header somewhere?

63 ↗

(On Diff #68193)

Can this static cast be lifted through the call chain to the entry point? Or are each of these used directly from table gen? This is more than a bit messy and concerning.

71 ↗

(On Diff #68193)

style: Is there a RegClass.max like value we could use here instead?

89 ↗

(On Diff #68193)

Not quite sure what we're going for here, but sign extending an unsigned immediate seems a bit odd. Possibly a clarifying comment?

101 ↗

(On Diff #68193)

Style: move this below the early return.

lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp

102 ↗

(On Diff #68193)

We need to assert the size of the immediate don't we? Also, didn't we have a checked cast function in one of the previous patches already?

104 ↗

(On Diff #68193)

Why the assert immediately followed by an unreachable

lib/Target/RISCV/RISCVInstrFormats.td

13 ↗

(On Diff #68193)

What is this? I don't see it being used anywhere in this patch?

[asb]

Refresh to reflect change made in previous patches in the series, and address review comments from @reames.

lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp

44 ↗

(On Diff #68193)

Actually this is redundant as they're already in RISCVMCTargetDesc.h which is already include - removed. Thanks!

63 ↗

(On Diff #68193)

I've lifted the cast to DecodeGPRRegisterClass, which is the entry-point from TableGen generated code, and in the end got rid of getReg entirely.

89 ↗

(On Diff #68193)

The immediate value is extracted from the instruction and passed as a uint64_t. The assert checks that it the bitwidth is as we expect, but we can sign-extend it from the appropriate bit position. SignExtend64 is written for this exact purpose (sign extends the bottom N bits of the uint64_t that it is passed).

I've added a comment to clarify what is going on that hopefully makes it easier to follow? I've also added a comment in the new decodeSImmOperandAndLsl1 which is perhaps even more confusing. Do let me know any suggestions for making it clearer.

lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp

102 ↗

(On Diff #68193)

I've now defined getImmOpValue rather than first defining getImm20OpValue only to change things in the next patch.

lib/Target/RISCV/RISCVInstrFormats.td

13 ↗

(On Diff #68193)

Tablegenning the disassembler fails without it. I've stolen an explanatory comment from the AMDGPU backend:

// SoftFail is a field the disassembler can use to provide a way for
// instructions to not match without killing the whole decode process. It is
// mainly used for ARM, but Tablegen expects this field to exist or it fails
// to build the decode table.

[theraven]

lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp

111 ↗

(On Diff #69356)

RISC-VC?

[jyknight]

test/MC/RISCV/rv32i-valid.s

3 ↗

(On Diff #69356)

Using the same file to test both directions seems like a nice method -- but strangely, almost unique amongst the tests for the backends.

I actually especially like the way it's done in test/MC/X86/mpx-encodings.s, where it uses the same CHECK: line for both the asm output of llvm-mc, and the output of llvm-objdump.

I'd suggest doing it that way, which would nicely resolve my comment in D23564 too.

[asb]

Tests now check both assembly printing and disassembly. I've added a comment/TODO regarding decode of instructions outside of the RV32/RV64 core that might be 16-bit, 48bit, 64bit, ...

[asb]

test/MC/RISCV/rv32i-valid.s

3 ↗

(On Diff #69356)

I went ahead with the CHECK-INST approach I described in D23564.

[asb]

Refresh to follow changes made in rL283702, putting TheRISCV32Target and TheRISCV64Target behind accessors.

[asb]

Fix rv32i-valid.s, so CHECK-INST is used rather than CHECK-DIS.

[asb]

The previous version of this patch made the mistake when decoding a GPR of assuming you can use the parsed RegNo as an index into the register class. In the general case, a register class will have the registers in preferred allocation order, meaning this will fail. Instead, we have a GPRDecoderTable to select the appropriate register based on the parsed RegNo.

llvm/trunk/lib/Target/RISCV/CMakeLists.txt

 set(LLVM_TARGET_DEFINITIONS RISCV.td)
 
 tablegen(LLVM RISCVGenRegisterInfo.inc -gen-register-info)
 tablegen(LLVM RISCVGenInstrInfo.inc -gen-instr-info)
 tablegen(LLVM RISCVGenMCCodeEmitter.inc -gen-emitter)
 tablegen(LLVM RISCVGenAsmMatcher.inc -gen-asm-matcher)
 tablegen(LLVM RISCVGenAsmWriter.inc -gen-asm-writer)
+tablegen(LLVM RISCVGenSubtargetInfo.inc -gen-subtarget)
+tablegen(LLVM RISCVGenDisassemblerTables.inc -gen-disassembler)
 
 add_public_tablegen_target(RISCVCommonTableGen)
 
 add_llvm_target(RISCVCodeGen
   RISCVTargetMachine.cpp
   )
 
 add_subdirectory(AsmParser)
+add_subdirectory(Disassembler)
 add_subdirectory(InstPrinter)
 add_subdirectory(MCTargetDesc)
 add_subdirectory(TargetInfo)

llvm/trunk/lib/Target/RISCV/Disassembler/CMakeLists.txt

+add_llvm_library(LLVMRISCVDisassembler
+  RISCVDisassembler.cpp
+  )

llvm/trunk/lib/Target/RISCV/Disassembler/LLVMBuild.txt

+;===- ./lib/Target/RISCV/Disassembler/LLVMBuild.txt ------------*- Conf -*--===;
+;
+;                     The LLVM Compiler Infrastructure
+;
+; This file is distributed under the University of Illinois Open Source
+; License. See LICENSE.TXT for details.
+;
+;===------------------------------------------------------------------------===;
+;
+; This is an LLVMBuild description file for the components in this subdirectory.
+;
+; For more information on the LLVMBuild system, please see:
+;
+;   http://llvm.org/docs/LLVMBuild.html
+;
+;===------------------------------------------------------------------------===;
+
+[component_0]
+type = Library
+name = RISCVDisassembler
+parent = RISCV
+required_libraries = MCDisassembler RISCVInfo Support
+add_to_library_groups = RISCV
+

llvm/trunk/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp

+//===-- RISCVDisassembler.cpp - Disassembler for RISCV --------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// This file implements the RISCVDisassembler class.
+//
+//===----------------------------------------------------------------------===//
+
+#include "MCTargetDesc/RISCVMCTargetDesc.h"
+#include "llvm/MC/MCContext.h"
+#include "llvm/MC/MCDisassembler/MCDisassembler.h"
+#include "llvm/MC/MCFixedLenDisassembler.h"
+#include "llvm/MC/MCInst.h"
+#include "llvm/MC/MCRegisterInfo.h"
+#include "llvm/MC/MCSubtargetInfo.h"
+#include "llvm/Support/Endian.h"
+#include "llvm/Support/TargetRegistry.h"
+
+using namespace llvm;
+
+#define DEBUG_TYPE "riscv-disassembler"
+
+typedef MCDisassembler::DecodeStatus DecodeStatus;
+
+namespace {
+class RISCVDisassembler : public MCDisassembler {
+
+public:
+  RISCVDisassembler(const MCSubtargetInfo &STI, MCContext &Ctx)
+      : MCDisassembler(STI, Ctx) {}
+
+  DecodeStatus getInstruction(MCInst &Instr, uint64_t &Size,
+                              ArrayRef<uint8_t> Bytes, uint64_t Address,
+                              raw_ostream &VStream,
+                              raw_ostream &CStream) const override;
+};
+} // end anonymous namespace
+
+static MCDisassembler *createRISCVDisassembler(const Target &T,
+                                               const MCSubtargetInfo &STI,
+                                               MCContext &Ctx) {
+  return new RISCVDisassembler(STI, Ctx);
+}
+
+extern "C" void LLVMInitializeRISCVDisassembler() {
+  // Register the disassembler for each target.
+  TargetRegistry::RegisterMCDisassembler(getTheRISCV32Target(),
+                                         createRISCVDisassembler);
+  TargetRegistry::RegisterMCDisassembler(getTheRISCV64Target(),
+                                         createRISCVDisassembler);
+}
+
+static const unsigned GPRDecoderTable[] = {
+  RISCV::X0_32,  RISCV::X1_32,  RISCV::X2_32,  RISCV::X3_32,
+  RISCV::X4_32,  RISCV::X5_32,  RISCV::X6_32,  RISCV::X7_32,
+  RISCV::X8_32,  RISCV::X9_32,  RISCV::X10_32, RISCV::X11_32,
+  RISCV::X12_32, RISCV::X13_32, RISCV::X14_32, RISCV::X15_32,
+  RISCV::X16_32, RISCV::X17_32, RISCV::X18_32, RISCV::X19_32,
+  RISCV::X20_32, RISCV::X21_32, RISCV::X22_32, RISCV::X23_32,
+  RISCV::X24_32, RISCV::X25_32, RISCV::X26_32, RISCV::X27_32,
+  RISCV::X28_32, RISCV::X29_32, RISCV::X30_32, RISCV::X31_32
+};
+
+static DecodeStatus DecodeGPRRegisterClass(MCInst &Inst, uint64_t RegNo,
+                                           uint64_t Address,
+                                           const void *Decoder) {
+   if (RegNo > sizeof(GPRDecoderTable)) {
+     return MCDisassembler::Fail;
+   }
+
+   // We must define our own mapping from RegNo to register identifier.
+   // Accessing index RegNo in the register class will work in the case that
+   // registers were added in ascending order, but not in general.
+   unsigned Reg = GPRDecoderTable[RegNo];
+   Inst.addOperand(MCOperand::createReg(Reg));
+   return MCDisassembler::Success;
+}
+
+template <unsigned N>
+static DecodeStatus decodeUImmOperand(MCInst &Inst, uint64_t Imm,
+                                      int64_t Address, const void *Decoder) {
+  assert(isUInt<N>(Imm) && "Invalid immediate");
+  Inst.addOperand(MCOperand::createImm(Imm));
+  return MCDisassembler::Success;
+}
+
+template <unsigned N>
+static DecodeStatus decodeSImmOperand(MCInst &Inst, uint64_t Imm,
+                                      int64_t Address, const void *Decoder) {
+  assert(isUInt<N>(Imm) && "Invalid immediate");
+  // Sign-extend the number in the bottom N bits of Imm
+  Inst.addOperand(MCOperand::createImm(SignExtend64<N>(Imm)));
+  return MCDisassembler::Success;
+}
+
+template <unsigned N>
+static DecodeStatus decodeSImmOperandAndLsl1(MCInst &Inst, uint64_t Imm,
+                                             int64_t Address,
+                                             const void *Decoder) {
+  assert(isUInt<N>(Imm) && "Invalid immediate");
+  // Sign-extend the number in the bottom N bits of Imm after accounting for
+  // the fact that the N bit immediate is stored in N-1 bits (the LSB is
+  // always zero)
+  Inst.addOperand(MCOperand::createImm(SignExtend64<N>(Imm << 1)));
+  return MCDisassembler::Success;
+}
+
+#include "RISCVGenDisassemblerTables.inc"
+
+DecodeStatus RISCVDisassembler::getInstruction(MCInst &MI, uint64_t &Size,
+                                               ArrayRef<uint8_t> Bytes,
+                                               uint64_t Address,
+                                               raw_ostream &OS,
+                                               raw_ostream &CS) const {
+  // TODO: although assuming 4-byte instructions is sufficient for RV32 and
+  // RV64, this will need modification when supporting the compressed
+  // instruction set extension (RVC) which uses 16-bit instructions. Other
+  // instruction set extensions have the option of defining instructions up to
+  // 176 bits wide.
+  Size = 4;
+  if (Bytes.size() < 4) {
+    Size = 0;
+    return MCDisassembler::Fail;
+  }
+
+  // Get the four bytes of the instruction.
+  uint32_t Inst = support::endian::read32le(Bytes.data());
+
+  return decodeInstruction(DecoderTable32, MI, Inst, Address, this, STI);
+}

llvm/trunk/lib/Target/RISCV/LLVMBuild.txt

 ;
 ; For more information on the LLVMBuild system, please see:
 ;
 ;   http://llvm.org/docs/LLVMBuild.html
 ;
 ;===------------------------------------------------------------------------===;
 
 [common]
-subdirectories = AsmParser InstPrinter TargetInfo MCTargetDesc
+subdirectories = AsmParser Disassembler InstPrinter TargetInfo MCTargetDesc
 
 [component_0]
 type = TargetGroup
 name = RISCV
 parent = Target
 has_asmparser = 1
 has_asmprinter = 1
+has_disassembler = 1
 
 [component_1]
 type = Library
 name = RISCVCodeGen
 parent = RISCV
 required_libraries = AsmPrinter Core CodeGen MC RISCVAsmPrinter RISCVDesc RISCVInfo Support Target
 add_to_library_groups = RISCV

llvm/trunk/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp

public:
   /// relocation, record the relocation and return zero.
   unsigned getMachineOpValue(const MCInst &MI, const MCOperand &MO,
                              SmallVectorImpl<MCFixup> &Fixups,
                              const MCSubtargetInfo &STI) const;
 
   unsigned getImmOpValueAsr1(const MCInst &MI, unsigned OpNo,
                              SmallVectorImpl<MCFixup> &Fixups,
                              const MCSubtargetInfo &STI) const;
+  unsigned getImmOpValue(const MCInst &MI, unsigned OpNo,
+                         SmallVectorImpl<MCFixup> &Fixups,
+                         const MCSubtargetInfo &STI) const;
 };
 } // end anonymous namespace
 
 MCCodeEmitter *llvm::createRISCVMCCodeEmitter(const MCInstrInfo &MCII,
                                               const MCRegisterInfo &MRI,
                                               MCContext &Ctx) {
   return new RISCVMCCodeEmitter(Ctx);
 }
RISCVMCCodeEmitter::getImmOpValueAsr1(const MCInst &MI, unsigned OpNo,
 
   if (MO.isImm()) {
     unsigned Res = MO.getImm();
     assert((Res & 1) == 0 && "LSB is non-zero");
     return Res >> 1;
   }
 
   llvm_unreachable("Unhandled expression!");
+
+  return 0;
+}
+
+unsigned RISCVMCCodeEmitter::getImmOpValue(const MCInst &MI, unsigned OpNo,
+                                           SmallVectorImpl<MCFixup> &Fixups,
+                                           const MCSubtargetInfo &STI) const {
+
+  const MCOperand &MO = MI.getOperand(OpNo);
+
+  // If the destination is an immediate, there is nothing to do
+  if (MO.isImm())
+    return MO.getImm();
+
+  llvm_unreachable("Unhandled expression!");
+
+  return 0;
 }
 
 #include "RISCVGenMCCodeEmitter.inc"

llvm/trunk/lib/Target/RISCV/MCTargetDesc/RISCVMCTargetDesc.h

 // Defines symbolic names for RISC-V registers.
 #define GET_REGINFO_ENUM
 #include "RISCVGenRegisterInfo.inc"
 
 // Defines symbolic names for RISC-V instructions.
 #define GET_INSTRINFO_ENUM
 #include "RISCVGenInstrInfo.inc"
 
+#define GET_SUBTARGETINFO_ENUM
+#include "RISCVGenSubtargetInfo.inc"
+
 #endif

llvm/trunk/lib/Target/RISCV/MCTargetDesc/RISCVMCTargetDesc.cpp

 #include "llvm/Support/TargetRegistry.h"
 
 #define GET_INSTRINFO_MC_DESC
 #include "RISCVGenInstrInfo.inc"
 
 #define GET_REGINFO_MC_DESC
 #include "RISCVGenRegisterInfo.inc"
 
+#define GET_SUBTARGETINFO_MC_DESC
+#include "RISCVGenSubtargetInfo.inc"
+
 using namespace llvm;
 
 static MCInstrInfo *createRISCVMCInstrInfo() {
   MCInstrInfo *X = new MCInstrInfo();
   InitRISCVMCInstrInfo(X);
   return X;
 }
 

 extern "C" void LLVMInitializeRISCVTargetMC() {
   for (Target *T : {&getTheRISCV32Target(), &getTheRISCV64Target()}) {
     TargetRegistry::RegisterMCAsmInfo(*T, createRISCVMCAsmInfo);
     TargetRegistry::RegisterMCInstrInfo(*T, createRISCVMCInstrInfo);
     TargetRegistry::RegisterMCRegInfo(*T, createRISCVMCRegisterInfo);
     TargetRegistry::RegisterMCAsmBackend(*T, createRISCVAsmBackend);
     TargetRegistry::RegisterMCCodeEmitter(*T, createRISCVMCCodeEmitter);
     TargetRegistry::RegisterMCInstPrinter(*T, createRISCVMCInstPrinter);
+    TargetRegistry::RegisterMCSubtargetInfo(*T, createRISCVMCSubtargetInfoImpl);
   }
 }

llvm/trunk/lib/Target/RISCV/RISCVInstrFormats.td

 //  a 21-bit value (where the LSB is always zero), we describe it as an imm20
 //  to match how it is encoded.
 //
 //===----------------------------------------------------------------------===//
 
 class RISCVInst<dag outs, dag ins, string asmstr, list<dag> pattern>
     : Instruction {
   field bits<32> Inst;
+  // SoftFail is a field the disassembler can use to provide a way for
+  // instructions to not match without killing the whole decode process. It is
+  // mainly used for ARM, but Tablegen expects this field to exist or it fails
+  // to build the decode table.
+  field bits<32> SoftFail = 0;
   let Size = 4;
 
   bits<7> Opcode = 0;
 
   let Inst{6-0} = Opcode;
 
   let Namespace = "RISCV";
 

llvm/trunk/lib/Target/RISCV/RISCVInstrInfo.td

def FenceArg : AsmOperandClass {
   let Name = "FenceArg";
   let RenderMethod = "addFenceArgOperands";
   let DiagnosticType = "InvalidFenceArg";
 }
 
 def fencearg : Operand<i32> {
   let ParserMatchClass = FenceArg;
   let PrintMethod = "printFenceArg";
+  let DecoderMethod = "decodeUImmOperand<4>";
 }
 
 def uimm5 : Operand<i32> {
   let ParserMatchClass = UImmAsmOperand<5>;
+  let DecoderMethod = "decodeUImmOperand<5>";
 }
 
 def simm12 : Operand<i32> {
   let ParserMatchClass = SImmAsmOperand<12>;
+  let DecoderMethod = "decodeSImmOperand<12>";
 }
 
 def uimm12 : Operand<i32> {
   let ParserMatchClass = UImmAsmOperand<12>;
+  let DecoderMethod = "decodeUImmOperand<12>";
 }
 
 // A 13-bit signed immediate where the least significant bit is zero.
 def simm13_lsb0 : Operand<i32> {
   let ParserMatchClass = SImmAsmOperand<13, "Lsb0">;
   let EncoderMethod = "getImmOpValueAsr1";
+  let DecoderMethod = "decodeSImmOperandAndLsl1<13>";
 }
 
 def uimm20 : Operand<i32> {
   let ParserMatchClass = UImmAsmOperand<20>;
+  let EncoderMethod = "getImmOpValue";
+  let DecoderMethod = "decodeUImmOperand<20>";
 }
 
 // A 21-bit signed immediate where the least significant bit is zero.
 def simm21_lsb0 : Operand<i32> {
   let ParserMatchClass = SImmAsmOperand<21, "Lsb0">;
   let EncoderMethod = "getImmOpValueAsr1";
+  let DecoderMethod = "decodeSImmOperandAndLsl1<21>";
 }
 
 // As noted in RISCVRegisterInfo.td, the hope is that support for
 // variable-sized register classes will mean that instruction definitions do
 // not need to be duplicated for 32-bit and 64-bit register classes. For now
 // we use 'GPR', which is 32-bit. When codegen for both RV32 and RV64 is
 // added, we will need to duplicate instruction definitions unless a proposal
 // like <http://lists.llvm.org/pipermail/llvm-dev/2016-September/105027.html>

llvm/trunk/test/MC/RISCV/rv32i-valid.s

 # RUN: llvm-mc %s -triple=riscv32 -show-encoding \
 # RUN:     | FileCheck -check-prefixes=CHECK,CHECK-INST %s
 # RUN: llvm-mc %s -triple=riscv64 -show-encoding \
 # RUN:     | FileCheck -check-prefixes=CHECK,CHECK-INST %s
+# RUN: llvm-mc -filetype=obj -triple riscv32 < %s \
+# RUN:     | llvm-objdump -d - | FileCheck -check-prefix=CHECK-INST %s
+# RUN: llvm-mc -filetype=obj -triple riscv64 < %s \
+# RUN:     | llvm-objdump -d - | FileCheck -check-prefix=CHECK-INST %s
 
 # CHECK-INST: lui a0, 2
 # CHECK: encoding: [0x37,0x25,0x00,0x00]
 lui a0, 2
 # CHECK-INST: lui s11, 552960
 # CHECK: encoding: [0xb7,0x0d,0x00,0x87]
 lui s11, (0x87000000>>12)
 # CHECK-INST: lui t0, 1048575