craig.topper added inline comments.
================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:4246
+  case Intrinsic::riscv_fsl:
+    return DAG.getNode(RISCVISD::FSL, DL, XLenVT, Op.getOperand(1),
+                       Op.getOperand(2), Op.getOperand(3));
----------------
craig.topper wrote:
> The operand order for RISCVISD::FSL/FSR match llvm.fshl and llvm.fshr rather 
> than rs1, rs2, rs3 order. I think the operand order you want for riscv.fsl 
> and riscv.fsr should be rs1, rs2, rs3.
> 
> And the assembly printing for fsl/fsr prints $rd, $rs1, $rs3, $rs2 makes this 
> even more confusing.
> 
> I'll put up a patch to change RISCVISD::FSL/FSR and RISCVISD::FSLW/FSRW order 
> to be in rs1, rs2, rs3 order.
i just noticed the proposed intrinsic order here 
https://github.com/riscv/riscv-bitmanip/blob/main-history/cproofs/rvintrin.h  
uses rs1, rs3, rs2/shamt ordering. Which is different than the order used in 
the spec pseudo code. But rs1, rs3, rs2/shamt matches how the instructions are 
printed.

So I guess we should use rs1, rs3, rs2/shamt order. I'll update the RISCVISD 
opcodes accordingly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117468

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

Reply via email to