sepavloff marked an inline comment as done. sepavloff 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 { ---------------- rjmccall wrote: > 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? There are 10 possible "arithmetic" rounding modes (https://upload.wikimedia.org/wikipedia/commons/8/8a/Comparison_rounding_graphs_SMIL.svg). Current implementation of glibc does not define anything beyond the standard. I know there are cores that use non-IEEE modes "stochastic rounding to nearest" and "away from zero", don't know if they expose these modes through standard interface like FLT_ROUNDS. It looks like there is no precedent of extending the value set returned by FLT_ROUNDS. 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