[PATCH] D91242: [RISCV] Custom lowering of SET_ROUNDING

[Serge Pavlov via Phabricator via llvm-commits]

sepavloff added a comment.

In D91242#2388630 <https://reviews.llvm.org/D91242#2388630>, @jrtc27 wrote:

> Would a SELECT not be more friendly to optimisations like known-bits analysis, or does that cause too much code to be emitted in the general case?

Not sure if I understand you correctly, if you mean analyzing the 3 bits of rounding mode bit-by-bit, It think it does not make sense here. It is unlikely that only a part of the bits are known. Rounding mode is specified as a constant or obtained as a result of FLT_ROUNDS or as argument of a function call. Usually it is not obtained by calculations. And yes, in the general case too much code is generated, at least 8 operators, while in this implementation 5 instructions is enough.



================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:225
 
+  if (Subtarget.hasStdExtD() || Subtarget.hasStdExtF()) {
+    setOperationAction(ISD::SET_ROUNDING, MVT::Other, Custom);
----------------
craig.topper wrote:
> StdExtD implies StdExtF. So I think you only need to check StdExtF
Fixed.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:906
+  if (auto CV = dyn_cast<ConstantSDNode>(RMValue)) {
+    uint64_t RM = cast<ConstantSDNode>(RMValue)->getZExtValue();
+    assert(RM <= static_cast<unsigned>(llvm::RoundingMode::MaxIEEEMode));
----------------
craig.topper wrote:
> Why do we need two code paths? Won't the other code constant fold if RMValue is a constant?
Indeed, the general case can be optimized for constant arguments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91242