[PATCH] D114581: [RISCV] Decode vtype with reserved fields to raw immediate

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Target/RISCV/MCTargetDesc/RISCVInstPrinter.cpp:176
+  if (RISCVVType::getVLMUL(Imm) == RISCVII::VLMUL::LMUL_RESERVED ||
+      RISCVVType::getSEW(Imm) > 64 || (Imm & 0x700) != 0) {
+    O << Imm;
----------------
jrtc27 wrote:
> Bit 10 doesn't exist in the immediate for vsetivli, though presumably it's always zero extended due to being a uimm so it doesn't matter. I'd have done `(Imm >> 8) != 0` instead though as it avoids needing to care about exactly how many bits to check, and matches the assert in printFenceArg.
> 
> This does also make the use of decodeUImmOperand<11> for vsetivli's operand a bit odd, but it does work given the N is just for an assert. When adding the immediate integer parsing support though you will discover that VTypeIAsmOperand, and thus VTypeIOp, need splitting so that you can ban 11-bit immediate arguments to vsetivli.
> Bit 10 doesn't exist in the immediate for vsetivli, though presumably it's always zero extended due to being a uimm so it doesn't matter. I'd have done `(Imm >> 8) != 0` instead though as it avoids needing to care about exactly how many bits to check, and matches the assert in printFenceArg.
> 

Changed to `Imm >> 8` in 622d6894801b19e795737e133f5963d248de45af



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114581/new/

https://reviews.llvm.org/D114581