StephenFan added inline comments.

================
Comment at: llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp:1663
 
+OperandMatchResultTy RISCVAsmParser::parseGPRasFPR(OperandVector &Operands) {
+  switch (getLexer().getKind()) {
----------------
StephenFan wrote:
> 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).
> 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>
----------------
StephenFan wrote:
> 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?
> 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?

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];
+    }
----------------
StephenFan wrote:
> 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
> 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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to