rjmccall added a reviewer: scanon.
rjmccall added inline comments.

================
Comment at: llvm/include/llvm/ADT/FloatingPointMode.h:26
+/// assigned to the rounding modes must agree with the values used by 
FLT_ROUNDS
+/// (C11, 5.2.4.2.2p8).
+enum class RoundingMode : int8_t {
----------------
sepavloff wrote:
> rjmccall wrote:
> > I agree that we should use one enum across LLVM and Clang.  I'm not sure 
> > that using the `FLT_ROUNDS` values is worthwhile, especially since (1) 
> > `FLT_ROUNDS` doesn't specify a value for some of these (like 
> > `NearestTiesToAway`) and (2) some of the values it does use (e.g. for 
> > "indeterminable") make this actively more awkward to store.  And the most 
> > useful thing we could do — matching the values of `FE_TONEAREST` and so on 
> > — isn't possible because those values are unportable.  I'd rather we just 
> > pick arbitrary, non-ABI-stable values, like we normally would, and then 
> > make the places that rely on matching some external schema translate.
> >  (1) FLT_ROUNDS doesn't specify a value for some of these (like 
> > NearestTiesToAway)
> 
> In the recent C standard draft 
> (http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2454.pdf), there is support 
> of all 5 rounding modes including values returned by `FLT_ROUNDS` 
> (5.2.4.2.2p11), values used by `fegetround` and `fesetround` 
> (FE_TONEARESTFROMZERO in 7.6p13)
> 
> >  (2) some of the values it does use (e.g. for "indeterminable") make this 
> > actively more awkward to store.
> 
> This is not a rounding mode value, it is just error indicator returned by 
> intrinsic functions, it does not need to be stored. Added comment about that.
> 
> > And the most useful thing we could do — matching the values of FE_TONEAREST 
> > and so on — isn't possible because those values are unportable.
> 
> I am working on patch that implements `fesetround` as intrinsic function. It 
> introduces two intrinsic functions, one is `llvm.set_rounding` (D74729 
> [FPEnv] Intrinsic for setting rounding mode) and the other is 
> `llvm.convert_rounding` (unpublished yet). The latter translates 
> platform-dependent values like FE_DOWNWARD into platform independent 
> representation, which is the same as used by FLT_ROUNDS.
> 
> Actually the motivation for this patch was just the need to have 
> platform-independent representation of rounding mode that could be used in 
> IR, which is more or less platform-independent. The representation used by 
> `FLT_ROUNDS` fits these purposes because:
> * it is platform-neutral,
> * it is defined by standard, and
> * it encodes all IEEE rounding modes. 
Okay.  I'm just worried that trying to match `FLT_ROUNDS` in our internal 
representation is ultimately going to cause unnecessary problems.  If C has 
standardized a value for NearesetTiesToAway, that certainly helps avoid that.  
`FLT_ROUNDS` allows targets to add implementation-defined values; are there any 
targets that support other rounding modes that aren't currently described?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77379



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

Reply via email to