[PATCH] D74730: [FPEnv][X86] Implement lowering of llvm.set.rounding

[Serge Pavlov via Phabricator via llvm-commits]

sepavloff added inline comments.


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:26971-26972
+    case RoundingMode::TowardZero:        FieldVal = X86::rmTowardZero; break;
+    default:
+      llvm_unreachable("rounding mode is not supported by X86 hardware");
+    }
----------------
lebedev.ri wrote:
> Is this enforced elsewhere? (verifier, etc)
> This doesn't seem like the right way do to error handling to me, we shouldn't crash.
It is not an error handling, it is an assertion check. This intrinsic is a low-level entity and is actually only a wrap over an instruction that writes to control register. It is responsibility of a caller to ensure that argument of this intrinsic is valid. A check in verifier is possible but  is not much helpful, because the argument may be a runtime expression.


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.h:855-856
+    /// Current rounding mode is represented in bits 11:10 of FPSR. These
+    /// values are same as corresponding constants for rounding mode used
+    /// in glibc.
+    enum RoundingMode {
----------------
lebedev.ri wrote:
> I suspect this is backwards, and should say that glibc choose to use values compatible with the ones stored in FPSR.
Updated the comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74730