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:
> > > 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.
> Okay, thanks, that's the kind of thing I meant. I have no objection to
> matching FLT_ROUNDS for the basic IEEE set of rounding modes that we have
> today — hey, we might as well — but we should acknowledge that fundamentally
> this is not the same as FLT_ROUNDS, and that places that need to generate a
> FLT_ROUNDS value may need a conversion, even if that conversion is likely
> very simple. For example, if targets with non-IEEE rounding modes wanted to
> support them in LLVM, we will just assign them the next available enumerators
> in `RoundingMode`, and they can separately make a policy decision about what
> they want to return from FLT_ROUNDS (either -1 or some implementation-defined
> value).
Yes, we can require identical rounding mode definitions only for standard
modes, mentioned in the standard. Fixed comment accordingly.
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
