[PATCH] D40003: [RISCV] MC layer support for the rest instructions of standard compress instruction set

[Alex Bradbury via Phabricator via llvm-commits]

asb added inline comments.


================
Comment at: lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp:281
+
+    if (isRV32Only(Insn, STI)) {
+      DEBUG(dbgs() << "Trying RISCV32Only_16 table (16-bit Instruction):\n");
----------------
Rather than introducing the isRV32only function which does its own minimal parsing and duplicates some information in Tablegen, can't we just use DecoderTableRISCV32Only_16 whenever we are targeting RV32, and then have Decoder16 as a fallback? Something like:

```
if (STI.getFeatureBits()[RISCV::Feature32Bit]) {
  DEBUG(dbgs() << "Trying RISCV32Only_16 table (16-bit Instruction):\n");
  // Calling the auto-generated decoder function.
  Result = decodeInstruction(DecoderTableRISCV32Only_16, MI, Insn, Address,
                             this, STI);
  if (Result != MCDisassembler::Fail) {
    Size = 2;
    return Result;
  }
}

DEBUG(dbgs() << "Trying RISCV_C table (16-bit Instruction):\n");
// Calling the auto-generated decoder function.
Result = decodeInstruction(DecoderTable16, MI, Insn, Address, this, STI);
```


================
Comment at: lib/Target/RISCV/RISCV.td:49
 def Feature64Bit
-    : SubtargetFeature<"64bit", "HasRV64", "true", "Implements RV64">;
+    : SubtargetFeature<"64bit", "RISCVArchVersion", "RV64", "Implements RV64">;
 def IsRV64 : Predicate<"Subtarget->is64Bit()">,
----------------
We might want to change the subtargetfeatures at some point, and introduce a new enum but I'd prefer to leave it with the simple boolean value in this patch. I can see how an enum might have some advantages, but I don't think the change belongs in this patch. Plus our HwModes are currently defined in terms of the presence or absence of Feature64Bit.

In the future, it may make sense to have a feature bit which indicates the base ISA (RV32I, RV32E, RV64I), particularly as it's possible there will be RV32EC-only encodings.


Repository:
  rL LLVM

https://reviews.llvm.org/D40003