[PATCH] D90854: [RISCV] Custom lowering of FLT_ROUNDS_

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:377
+    setOperationAction(ISD::FLT_ROUNDS_,
+                       (Subtarget.is64Bit() ? MVT::i64 : MVT::i32), Custom);
+  }
----------------
You can use XLenVT instead of picking i64/i32.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:4113
+                                               SelectionDAG &DAG) const {
+  const MVT IntTy = Subtarget.getXLenVT();
+  SDLoc DL(Op);
----------------
Rename IntTy to XLenVT to match what most of the RISCV back does.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:4117
+  SDValue SysRegNo =
+      DAG.getConstant(static_cast<int>(RISCV::SysReg::FRM), DL, IntTy);
+  SDVTList VTs = DAG.getVTList(IntTy, MVT::Other);
----------------
Use `RISCVSysReg::lookupSysRegByName("FRM")->Encoding` it's not common enough to justify having the encoding in 2 places.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:4620
+  case ISD::FLT_ROUNDS_: {
+    assert(N->getValueType(0) == MVT::i32 && Subtarget.is64Bit() &&
+           Subtarget.hasStdExtF() && "Unexpected custom legalization");
----------------
Is this code reachable? This would require `setOperaction(ISD::FLT_ROUNDS_, MVT::i32, Custom)` when Subtarget.is64Bit() is false.

It's also the default behavior for the target independent type legalizer.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.h:266
+
+enum Rounding {
+  RNE = 0, ///< roundTiesToEven.
----------------
Can you use RISCVFPRndMode from lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.h


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90854