[PATCH] D93298: [RISCV] add the MC layer support of Zfinx extension

[luxufan via Phabricator via llvm-commits]

StephenFan added inline comments.


================
Comment at: llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp:1663
 
+OperandMatchResultTy RISCVAsmParser::parseGPRasFPR(OperandVector &Operands) {
+  switch (getLexer().getKind()) {
----------------
jrtc27 wrote:
> Why can't you just use parseRegister?
use the default parseRegister will make the test cases in other files fail. For example:

```
fcvt.d.l a3, ft3 # CHECK: :[[@LINE]]:10: error: invalid operand for instruction
```
this is the test case in rv64d-invalid.s. If uses the default parseRegister. the invalid operand is in column 14 (ft3 operand) instead of 10 (a3 operand).


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoZfinx.td:59
+let hasSideEffects = 0, mayLoad = 0, mayStore = 0 in
+class FPUnaryOpINX_r<bits<7> funct7, bits<3> funct3, RegisterOperand rdty,
+                RegisterOperand rs1ty, string opcodestr>
----------------
jrtc27 wrote:
> Don't duplicate all these, they're identical to the normal floating point versions.
Because of normal floating point version only support RegisterClass, but we use the RegisterOperand, so we change this. Or if there is more convenient way to resolve this?


================
Comment at: llvm/lib/Target/RISCV/RISCVRegisterInfo.td:468
+                       Reg.AltNames> {
+      let SubRegIndices = [sub_32, sub_32_hi];
+    }
----------------
jrtc27 wrote:
> Does this hard-coding of 32 cause issues on RV64?
I don't known if it will cause issues on RV64. But the zfinx spec specifies that register pairs are only used in RV32


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

https://reviews.llvm.org/D93298