[PATCH] D42782: [RISCV] Fix c.addi and c.addi16sp immediate constraints which should be non-zero

[Wei-Ren Chen via Phabricator via llvm-commits]

chenwj added a comment.

Comment inline.



================
Comment at: lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp:358
     bool IsConstantImm = evaluateConstantImm(Imm, VK);
-    return IsConstantImm && isShiftedInt<6, 4>(Imm) &&
+    return IsConstantImm && isShiftedInt<6, 4>(Imm) && (Imm != 0) &&
            VK == RISCVMCExpr::VK_RISCV_None;
----------------
Nitpick. Wouldn't be better testing Imm is zero once we know Imm is a constant?

```
return IsConstantImm && (Imm != 0) && isShiftedInt<6, 4>(Imm) &&
            VK == RISCVMCExpr::VK_RISCV_None;
```


================
Comment at: lib/Target/RISCV/RISCVInstrInfoC.td:103
+                            ImmLeaf<XLenVT,
+                            [{return isShiftedInt<6, 4>(Imm) && (Imm != 0);}]> {
+  let ParserMatchClass = SImmAsmOperand<10, "Lsb0000NonZero">;
----------------
Same here.


Repository:
  rL LLVM

https://reviews.llvm.org/D42782