Page MenuHomePhabricator

[FPEnv] Use single enum to represent rounding mode
ClosedPublic

Authored by sepavloff on Apr 3 2020, 3:43 AM.

Details

Summary

Now compiler defines 5 sets of constants to represent rounding mode.
These are:

  1. llvm::APFloatBase::roundingMode. It specifies all 5 rounding modes

defined by IEEE-754 and is used in APFloat implementation.

  1. clang::LangOptions::FPRoundingModeKind. It specifies 4 of 5 IEEE-754

rounding modes and a special value for dynamic rounding mode. It is used
in clang frontend.

  1. llvm::fp::RoundingMode. Defines the same values as

clang::LangOptions::FPRoundingModeKind but in different order. It is
used to specify rounding mode in IR and functions that operate IR.

  1. Rounding mode representation used by FLT_ROUNDS (C11, 5.2.4.2.2p7).

Besides constants for rounding mode it also defines a special value to
indicate errors. It is convenient to use in intrinsic functions, as it
is a platform-independent representation for rounding mode. In this
role it is used in some pending patches.

  1. Values like FE_DOWNWARD and other, which specify rounding mode in

library calls fesetround and fegetround. Often they represent bits
of some control register, so they are target-dependent. The same names
(not values) and a special name FE_DYNAMIC are used in
#pragma STDC FENV_ROUND.

The first 4 sets of constants are target independent and could have the
same numerical representation. It would simplify conversion between the
representations. Also now clang::LangOptions::FPRoundingModeKind and
llvm::fp::RoundingMode do not contain the value for IEEE-754 rounding
direction roundTiesToAway, although it is supported natively on
some targets.

This change defines all the rounding mode types via one enumeration
llvm::RoundingMode, which also contains rounding mode for IEEE
rounding direction roundTiesToAway. This enumeration uses the encoding
specified by C standard for FLT_ROUNDS, as it is the only representation
in which numerical values of rounding modes is fixed.

Diff Detail

Event Timeline

sepavloff created this revision.Apr 3 2020, 3:43 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 3 2020, 3:44 AM
rjmccall added inline comments.Apr 3 2020, 3:33 PM
llvm/include/llvm/ADT/FloatingPointMode.h
26

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.

sepavloff updated this revision to Diff 255267.Apr 6 2020, 2:48 AM

Updated patch

  • enhancements of LangOptions.def are moved into separate patch,
  • fixed some clang-format errors,
  • added more comments to RoundingMode.
sepavloff marked an inline comment as done.Apr 6 2020, 3:10 AM
sepavloff added inline comments.
llvm/include/llvm/ADT/FloatingPointMode.h
26
(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.
rjmccall added inline comments.
llvm/include/llvm/ADT/FloatingPointMode.h
26

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?

sepavloff marked an inline comment as done.Apr 7 2020, 12:42 AM
sepavloff added inline comments.
llvm/include/llvm/ADT/FloatingPointMode.h
26

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.

rjmccall added inline comments.Apr 7 2020, 9:44 AM
llvm/include/llvm/ADT/FloatingPointMode.h
26

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).

sepavloff updated this revision to Diff 255710.Apr 7 2020, 10:10 AM

Removed dependency

sepavloff updated this revision to Diff 255949.Apr 8 2020, 3:23 AM

Updated patch

  • Fixed comment,
  • Mention "round.tonearestaway" in documentation.
sepavloff marked an inline comment as done.Apr 8 2020, 3:26 AM
sepavloff added inline comments.
llvm/include/llvm/ADT/FloatingPointMode.h
26

Yes, we can require identical rounding mode definitions only for standard modes, mentioned in the standard. Fixed comment accordingly.

rjmccall accepted this revision.Apr 8 2020, 9:16 AM

Okay. I can accept this.

This revision is now accepted and ready to land.Apr 8 2020, 9:16 AM
This revision was automatically updated to reflect the committed changes.