[PATCH] D148519: [RISCV] Support vector strict rounding operations.

Craig Topper via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Apr 18 15:50:30 PDT 2023


craig.topper added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:401
                         ISD::STRICT_FROUND, ISD::STRICT_FROUNDEVEN,
-                        ISD::STRICT_FTRUNC},
+                        ISD::STRICT_FTRUNC, ISD::STRICT_FNEARBYINT},
                        MVT::f16, Promote);
----------------
STRICT_FNEARBYINT is already the third entry in the list


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:2550
+  SDValue Chain = Op.getOperand(0);
+  SDValue Src = Op.getOperand(1);
+
----------------
Src needs a freeze.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:2556
+                  Chain, Src, Src, DAG.getCondCode(ISD::SETUO));
+  SDValue Fadd = DAG.getNode(ISD::STRICT_FADD, DL,
+                             DAG.getVTList(VT, MVT::Other), Chain, Src, Src);
----------------
This needs to be a masked fadd I think. If Src is a large number instead of a nan, adding a larger number to itself can produce infinity which will set the overflow exception which we don't want.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:2583
+  }
+  SDValue BaseRes = DAG.getNode(BaseOpc, DL, VT, Src);
+  Chain = DAG.getMergeValues({Unorder.getValue(1), Fadd.getValue(1)}, DL);
----------------
I'm not sure we can convert to the base opcode. I think we need to keep the chain through the expanded opcodes. We need to make sure rounding mode changes or reads of fflags that should logically happen after the rounding expansion can't move up. The fadd probably protects anything above from sinking down.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148519



More information about the llvm-commits mailing list