Page MenuHomePhabricator

Fix interaction of pragma FENV_ACCESS with other pragmas
ClosedPublic

Authored by sepavloff on May 25 2022, 3:11 AM.

Details

Summary

Previously #pragma STDC FENV_ACCESS ON always set dynamic rounding
mode and strict exception handling. It is not correct in the presence
of other pragmas that also modify rounding mode and exception handling.
For example, the effect of previous pragma FENV_ROUND could be
cancelled, which is not conformant with the C standard. Also
#pragma STDC FENV_ACCESS OFF turned off only FEnvAccess flag, leaving
rounding mode and exception handling unchanged, which is incorrect in
general case.

Concrete rounding and exception mode depend on a combination of several
factors like various pragmas and command-line options. During the review
of this patch an idea was proposed that the semantic actions associated
with such pragmas should only set appropriate flags. Actual rounding
mode and exception handling should be calculated taking into account the
state of all relevant options. In such implementation the pragma
FENV_ACCESS should not override properties set by other pragmas but
should set them if such setting is absent.

To implement this approach the following main changes are made:

  • Field FPRoundingMode is removed from LangOptions. Actually there are no options that set it to arbitrary rounding mode, the choice was only dynamic or tonearest. Instead, a new boolean flag RoundingMath is added, with the same meaning as the corresponding command-line option.
  • Type FPExceptionModeKind now has possible value FPE_Default. It does not represent any particular exception mode but indicates that such mode was not set and default value should be used. It allows to distinguish the case:

    { #pragma STDC FENV_ACCESS ON ... }

    where the pragma must set FPE_Strict, from the case:

    { #pragma clang fp exceptions(ignore) #pragma STDC FENV_ACCESS ON ... }

    where exception mode should remain FPE_Ignore.
    • Class FPOptions has now methods getRoundingMode and getExceptionMode, which calculates the respective properties from other specified FP properties.
    • Class LangOptions has now methods getDefaultRoundingMode and getDefaultExceptionMode, which calculates default modes from the specified options and should be used instead of getRoundingMode and getFPExceptionMode of the same class.

Diff Detail

Event Timeline

sepavloff created this revision.May 25 2022, 3:11 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 25 2022, 3:11 AM
sepavloff requested review of this revision.May 25 2022, 3:11 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 25 2022, 3:11 AM

Could you lay out the expected interaction between "STDC FENV_ACCESS", "clang fp exceptions", "float_control", and "fenv_access"? If there's some way to map everything to "#pragma clang fp", please lay that out; if that isn't possible, please explain why.

As far as I can tell, "STDC FENV_ACCESS" and "STDC FENV_ROUND" don't directly interact. FENV_ROUND just overrides the rounding mode for specific floating-point operations; it doesn't impact whether environment access is allowed. So the call to setRoundingModeOverride seems dubious.

Could you lay out the expected interaction between "STDC FENV_ACCESS", "clang fp exceptions", "float_control", and "fenv_access"? If there's some way to map everything to "#pragma clang fp", please lay that out; if that isn't possible, please explain why.

pragma fenv_access is same as pragma STDC FENV_ACCESS.
pragma float_control(except, ...) is same as pragma clang fp exception amended with possibility to maintain stack of the directives using push token.
So indeed, all cases can be mapped to the interaction of pragma STDC FENV_ACCESS with pragma clang fp exception.
Possible combinations are:

  1. No access to FP environment:
	#pragma STDC FENV_ACCESS OFF
	#pragma clang fp exception(ignore)  // maytrap or strict is not allowed
  1. Strict exception mode:
	#pragma STDC FENV_ACCESS ON
	#pragma clang fp exception(strict)
  1. MayTrap exception mode:
	#pragma STDC FENV_ACCESS ON
	#pragma clang fp exception(maytrap)
  1. Ignore exception mode
	#pragma STDC FENV_ACCESS ON
	#pragma clang fp exception(ignore)

In this mode the code may change FP control modes, like rounding mode, but FP exceptions are ignored.
All of them must be supported.

As far as I can tell, "STDC FENV_ACCESS" and "STDC FENV_ROUND" don't directly interact. FENV_ROUND just overrides the rounding mode for specific floating-point operations; it doesn't impact whether environment access is allowed. So the call to setRoundingModeOverride seems dubious.

According to the latest standard draft http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2731.pdf:

7.6.2p2
The FENV_ROUND pragma provides a means to specify a constant rounding direction for floating-point
operations for standard floating types ...

7.6.2p4
... Within the scope of an FENV_ROUND pragma establishing a mode other than FE_DYNAMIC, floating-point operators, implicit conversions ..., and invocations of functions indicated in the table below, ...  shall be evaluated according to the specified constant rounding mode (as though no constant mode was specified and the corresponding dynamic rounding mode had been established by a call to fesetround).

So indeed the pragmas are independent, with the exception:

7.6.2p3
... If the FE_DYNAMIC mode is specified and FENV_ACCESS is "off", the translator may assume that the default rounding mode is in effect.

If target allows specifying rounding mode in instruction (like RISCV), FENV_ROUND may be implemented without changing FP environment. However for targets, where change of rounding mode requires modification of a control register, there is no other way. The standard explicitly allows it in:

7.6.2p5
NOTE Constant rounding modes (other than FE_DYNAMIC) could be implemented using dynamic rounding modes as
illustrated in the following example:
{
    #pragma STDC FENV_ROUND direction
    // compiler inserts:
    // #pragma STDC FENV_ACCESS ON
    // int __savedrnd;
    // __savedrnd = __swapround(direction);
    ... operations affected by constant rounding mode ...
    // compiler inserts:
    // __savedrnd = __swapround(__savedrnd);
    ... operations not affected by constant rounding mode ...
    // compiler inserts:
    // __savedrnd = __swapround(__savedrnd);
    ... operations affected by constant rounding mode ...
    // compiler inserts:
    // __swapround(__savedrnd);
}

where __swapround is defined by:

static inline int __swapround(const int new) {
    const int old = fegetround();
    fesetround(new);
    return old;
}

As it follows from the standard, FENV_ROUND in general case changes FP environment (sets rounding mode). Also it affects any FP operation in the scope of the pragma, which is modelled as current rounding mode in clang. So the call to setRoundingModeOverride seems necessary.

For FENV_ROUND, I think we should try to stick to the standard as closely as possible in Sema; since the standard models FENV_ROUND as a separate state, Sema should also model it as a separate state. If CodeGen decides it needs to modify the FP environment to actually implement the standard's rules, it can change the way it generates code to match. (The example in 7.6.2p5 is just pseudo-code; any internal state changes should be transparent to the user.)

For FENV_ROUND, I think we should try to stick to the standard as closely as possible in Sema; since the standard models FENV_ROUND as a separate state, Sema should also model it as a separate state.

I try to figure out the difference between rounding mode as specified by FENV_ROUND and that represented by Sema::CufFPFeatures. The standard uses two notions for rounding mode:

  • dynamic rounding mode, which is actually bits in a control register, and
  • constant rounding mode, which is used for floating-point operators, implicit conversions and so on.

Dynamic rounding mode is set/queried by fegetround/fesetround/FLT_ROUNDS. It is unknown at compile time except that in default FP mode it is expected to be FE_TONEAREST.

What compiler can know is constant rounding mode, it is just the mode set by pragma FENV_ROUND.

It looks like Sema::CufFPFeatures is exactly what FENV_ROUND controls.
Do I miss something?

I mean that ActOnPragmaFEnvAccess shouldn't call hasRoundingModeOverride()/setRoundingModeOverride(), and setRoundingMode shouldn't call getAllowFEnvAccess().

I mean that ActOnPragmaFEnvAccess shouldn't call hasRoundingModeOverride()/setRoundingModeOverride(), and setRoundingMode shouldn't call getAllowFEnvAccess().

setRoundingMode definitely should not call getAllowFEnvAccess() and it does not. As for ActOnPragmaFEnvAccess, it make sense to set the most general mode. Code like:

{
#pragma STDC FENV_ACCESS ON
    fesetround(x);
    ...
}

must work as is, and it requires that operations are created with dynamic rounding mode.

If a user wants to have more specific mode, they can amend FENV_ACCESS with other pragmas, like FENV_ROUND or clang fp exceptions.

setRoundingMode definitely should not call getAllowFEnvAccess() and it does not

Yes, it does?

// C2x: 7.6.2p3  If the FE_DYNAMIC mode is specified and FENV_ACCESS is "off",
// the translator may assume that the default rounding mode is in effect.
if (FPR == llvm::RoundingMode::Dynamic &&
    !CurFPFeatures.getAllowFEnvAccess() &&
    CurFPFeatures.getFPExceptionMode() == LangOptions::FPE_Ignore)
  FPR = llvm::RoundingMode::NearestTiesToEven;

As for ActOnPragmaFEnvAccess, it make sense to set the most general mode.

Shouldn't the default mode just be "dynamic"? The fact that we emit non-strict floating-point ops if FENV_ACCESS is disabled is an implementation detail. I don't see the point of switching the rounding mode between "Dynamic" and "NearestTiesToEven" depending on whether FENV_ACCESS is enabled. Making the required state transitions complicated like this just makes the code harder to understand.

setRoundingMode definitely should not call getAllowFEnvAccess() and it does not

Yes, it does?

// C2x: 7.6.2p3  If the FE_DYNAMIC mode is specified and FENV_ACCESS is "off",
// the translator may assume that the default rounding mode is in effect.
if (FPR == llvm::RoundingMode::Dynamic &&
    !CurFPFeatures.getAllowFEnvAccess() &&
    CurFPFeatures.getFPExceptionMode() == LangOptions::FPE_Ignore)
  FPR = llvm::RoundingMode::NearestTiesToEven;

Sorry for my confusion, I meant setAllowFEnvAccess().

As for ActOnPragmaFEnvAccess, it make sense to set the most general mode.

Shouldn't the default mode just be "dynamic"? The fact that we emit non-strict floating-point ops if FENV_ACCESS is disabled is an implementation detail. I don't see the point of switching the rounding mode between "Dynamic" and "NearestTiesToEven" depending on whether FENV_ACCESS is enabled. Making the required state transitions complicated like this just makes the code harder to understand.

The standard says that

the translator may assume that the default rounding mode is in effect

so it does not require to make such assumption, and we can use the most profitable way.

The advantages of using default rounding mode are:

  1. In this case the mode has concrete value. Constant evaluator can evaluate expressions like 1.0/3.0, which is not allowed if rounding is dynamic, and the expression is not constexpr anymore.
  1. On targets that support static rounding mode (like RISCV) dynamic and constant rounding modes may be different and the behavior changes if default mode is replaced by dynamic.

In these cases choosing dynamic mode is not an implementation details, it has user-visible effect and the behavior should be chosen most close to the standard.

It seems that #pragma STDC FENV_ROUND FE_DYNAMIC without #pragma STDC FENV_ACCESS ON is more like an error, because there is no legitimate way to set rounding mode in this case.

The way I see it, there are two possibilities here:

  1. In Sema, we have two rounding modes that correspond to FE_DYNAMIC: llvm::RoundingMode::Dynamic, and llvm::RoundingMode::NearestTiesToEven, plus some boolean to indicate whether the user actually explicitly specified FE_TONEAREST. (You're currently missing the boolean, which means that currently "#pragma STDC FENV_ACCESS OFF" followed by "#pragma STDC FENV_ACCESS ON" actually mutates the rounding mode.) We juggle the rounding modes and the bit based on whether FENV_ACCESS is currently enabled.
  2. We just have one rounding mode in Sema that corresponds to FE_DYNAMIC. Then in CodeGen, we set the actual rounding mode based on whether FENV_ACCESS is currently enabled.

(2) seems a lot simpler.

On targets that support static rounding mode (like RISCV) dynamic and constant rounding modes may be different and the behavior changes if default mode is replaced by dynamic.

Whether a target supports static rounding modes on floating-point instructions is completely irrelevant. The user-visible behavior must be the same either way. If a target doesn't have specialized instructions, the code generator can save/restore the rounding mode. This should be transparent to the user; the user can't read the rounding mode in between the save and restore. (We already do this sort of rounding mode manipulation on x86, to implement float-to-int conversion on targets without SSE.)

kpn added a comment.Jun 1 2022, 6:42 AM

On targets that support static rounding mode (like RISCV) dynamic and constant rounding modes may be different and the behavior changes if default mode is replaced by dynamic.

Whether a target supports static rounding modes on floating-point instructions is completely irrelevant. The user-visible behavior must be the same either way. If a target doesn't have specialized instructions, the code generator can save/restore the rounding mode. This should be transparent to the user; the user can't read the rounding mode in between the save and restore. (We already do this sort of rounding mode manipulation on x86, to implement float-to-int conversion on targets without SSE.)

The rounding mode argument to the constrained intrinsic is a hint; it tells the intrinsic what rounding mode has been set via the normal rounding mode changing mechanism. Constrained intrinsics don't change the rounding mode. I guess we could weaken that by adding "... except if absolutely required" to account for the float-to-int sans SSE case, but the principle applies.

Because of the requirement that the rounding mode be changed already, I don't see how instructions with static rounding modes are generally interesting. Perhaps a target will learn to recognize that a sequence of instructions can use the static rounding in the instructions and elide a change of rounding mode around the sequence? I don't know how common that would be in practice.

Constrained intrinsics don't change the rounding mode.

The standard text for FENV_ROUND requires that when we see a floating point operation, we have to perform it using the specified rounding mode. "floating-point operators [...] shall be evaluated according to the specified constant rounding mode (as though no constant mode was specified and the corresponding dynamic rounding mode had been established by a call to fesetround)". Like you've noted, we don't have any way to represent that directly in LLVM IR. So the frontend probably needs to emit code to modify the rounding mode.

Because of the requirement that the rounding mode be changed already, I don't see how instructions with static rounding modes are generally interesting. Perhaps a target will learn to recognize that a sequence of instructions can use the static rounding in the instructions and elide a change of rounding mode around the sequence? I don't know how common that would be in practice.

If you're using FENV_ROUND, presumably sequences involving rounding mode changes become more common.

sepavloff updated this revision to Diff 433963.Jun 2 2022, 11:23 PM

Update the patch

  • Add a flag to reflect rounding mode change by FENV_ROUND,
  • Keep dynamic mode in AST and change it later in CodeGen,
  • Add new tests.

The way I see it, there are two possibilities here:

  1. In Sema, we have two rounding modes that correspond to FE_DYNAMIC: llvm::RoundingMode::Dynamic, and llvm::RoundingMode::NearestTiesToEven, plus some boolean to indicate whether the user actually explicitly specified FE_TONEAREST. (You're currently missing the boolean, which means that currently "#pragma STDC FENV_ACCESS OFF" followed by "#pragma STDC FENV_ACCESS ON" actually mutates the rounding mode.) We juggle the rounding modes and the bit based on whether FENV_ACCESS is currently enabled.
  2. We just have one rounding mode in Sema that corresponds to FE_DYNAMIC. Then in CodeGen, we set the actual rounding mode based on whether FENV_ACCESS is currently enabled.

(2) seems a lot simpler.

It makes sense. Updated the patch accordingly.

Given we have getEffectiveRoundingMode(), I think the calls to setRoundingModeOverride shouldn't be necessary? And if we drop those calls, we can also drop the IsRoundingModeSet variable.

Given we have getEffectiveRoundingMode(), I think the calls to setRoundingModeOverride shouldn't be necessary? And if we drop those calls, we can also drop the IsRoundingModeSet variable.

In the block:

{
#pragma STDC FENV_ACCESS ON
    fesetround(x);
    ...
}

floating point operations must have rounding mode dynamic. If handler of FENV_ACCESS does not set it, the rounding mode remains FE_TONEAREST, which is incorrect.

Shouldn't the rounding mode be FE_DYNAMIC by default?

Shouldn't the rounding mode be FE_DYNAMIC by default?

According to the standard it must be FE_TONEAREST:

F.8.3p1:
At program startup the dynamic floating-point environment is initialized as prescribed by IEC 60559:
...
- The dynamic rounding direction mode is rounding to nearest.
...

Shouldn't the rounding mode be FE_DYNAMIC by default?

According to the standard it must be FE_TONEAREST:

F.8.3p1:
At program startup the dynamic floating-point environment is initialized as prescribed by IEC 60559:
...
- The dynamic rounding direction mode is rounding to nearest.
...

When I say the default should be FE_DYNAMIC, I'm talking about the default FENV_ROUND state. That's talking about the initial state of the floating-point registers at runtime (on entry to main()).

Shouldn't the rounding mode be FE_DYNAMIC by default?

According to the standard it must be FE_TONEAREST:

F.8.3p1:
At program startup the dynamic floating-point environment is initialized as prescribed by IEC 60559:
...
- The dynamic rounding direction mode is rounding to nearest.
...

When I say the default should be FE_DYNAMIC, I'm talking about the default FENV_ROUND state. That's talking about the initial state of the floating-point registers at runtime (on entry to main()).

The cited excerpt from the standard is just about floating point environment at program startup.

efriedma added inline comments.Jun 6 2022, 11:45 AM
clang/include/clang/Basic/LangOptions.h
658–659

I'm suggesting this should be setRoundingMode(llvm::RoundingMode::Dynamic).

If FENV_ACCESS is off, getEffectiveRoundingMode() converts that to "nearest". If FENV_ACCESS is turned on, the mode is treated as dynamic. This is exactly what we want.

sepavloff added inline comments.Jun 6 2022, 11:56 AM
clang/include/clang/Basic/LangOptions.h
658–659

It would change semantics. In particular, it would make impossible to use FP arithmetic in constexpr functions. An expression like 1.0 / 3.0 cannot be evaluated with dynamic rounding mode.

efriedma added inline comments.Jun 6 2022, 12:05 PM
clang/include/clang/Basic/LangOptions.h
658–659

Can we just change the relevant code in ExprConstant to call getEffectiveRoundingMode()?

sepavloff added inline comments.Jun 7 2022, 4:11 AM
clang/include/clang/Basic/LangOptions.h
658–659

What is the advantage of using FE_DYNAMIC in the case when it is known that rounding mode is FE_TONEAREST?

Probably FENV_ROUND FE_DYNAMIC should result in dynamic rounding even if FENV_ACCESS ON is absent. Rounding mode cannot be changed in this case but it can be used to inform the compiler that such function can be called in environment, where rounding mode is non-default. It would prevent some constant evaluations and other transformations that assume rounding mode is known. Anyway the standard only allow to assume FE_TONEAREST but does not require this.

In such case getEffectiveRoundingMode is not needed.

efriedma added inline comments.Jun 7 2022, 9:51 AM
clang/include/clang/Basic/LangOptions.h
658–659

I really want to keep the state we store, and the state updates, as simple as possible; if that means making getEffectiveRoundingMode() or whatever more complicated, that's fine. It's a matter of making sure we understand all the relevant transitions.

As far as I can tell, according to the standard, the initial state for FENV_ROUND is supposed to be FE_DYNAMIC, as if "#pragma STDC FENV_ROUND FE_DYNAMIC" were written at the beginning of every file. If we think we need a new state, something like FE_DYNAMIC_INITIAL, to represent the initial state, we can, I guess, but I don't think the standard text requires it.

sepavloff added inline comments.Jun 7 2022, 10:11 AM
clang/include/clang/Basic/LangOptions.h
658–659

As far as I can tell, according to the standard, the initial state for FENV_ROUND is supposed to be FE_DYNAMIC, as if "#pragma STDC FENV_ROUND FE_DYNAMIC" were written at the beginning of every file.

Could you please give any reference to this statement? I thought the initial state should be FE_TONEAREST, as it follows from F.8.3p1.

efriedma added inline comments.Jun 7 2022, 10:38 AM
clang/include/clang/Basic/LangOptions.h
658–659

" If no FENV_ROUND pragma is in effect, or the specified constant rounding mode is FE_DYNAMIC, rounding is according to the mode specified by the dynamic floating-point environment". And all the other rules for FENV_ROUND only apply to "an FENV_ROUND pragma establishing a mode other than FE_DYNAMIC". So no FENV_ROUND is equivalent to "FENV_ROUND FENV_DYNAMIC".

The text also says, "If the FE_DYNAMIC mode is specified and FENV_ACCESS is 'off', the translator may assume that the default rounding mode is in effect". But that's the same result you'd get if you didn't specify FENV_ROUND at all: it's just combining the general rule that you're not allowed to mess with the dynamic rounding mode outside the scope of FENV_ACCESS, with the rule that the initial rounding mode is "nearest".

sepavloff added inline comments.Jun 8 2022, 9:40 AM
clang/include/clang/Basic/LangOptions.h
658–659

Thank you for the references. Indeed, default behavior specified by the standard is to use dynamic rounding. It however do not agree with the default compiler behavior - to use FE_TONEAREST. That's why we cannot make setRoundingMode(llvm::RoundingMode::Dynamic).

With options '-frounding-math' compiler conforms to the latest standard draft. In this case however FENV_ACCESS is not necessary. Using FE_DYNAMIC with subsequent deduction of actual rounding mode looks a fragile solution compared with setting actual mode in CurFPFeatures.

efriedma added inline comments.Jun 9 2022, 1:27 PM
clang/include/clang/Basic/LangOptions.h
658–659

My goal is to make the state transitions as simple as possible. It's very easy to mess up state machines. This means as few variables as possible, in as few states as possible, are involved in parsing the relevant pragmas.

It however do not agree with the default compiler behavior - to use FE_TONEAREST.

My goal is to make the implementations of ActOnPragmaFEnvAccess and ActOnPragmaFEnvRound as simple as possible. So the question is, if we store the state of FENV_ACCESS as a boolean, and the state of FENV_ROUND as a rounding mode that defaults to FE_DYNAMIC, do we have enough information to produce correct code? I think we do, and you haven't given a counterexample.

I'm less concerned about how complicated it is to compute any state derived from those variables.

Remade the patch according to the review notes

sepavloff added inline comments.Jun 17 2022, 12:24 AM
clang/include/clang/Basic/LangOptions.h
658–659

I see your point. The patch is remade according to this goal.

sepavloff edited the summary of this revision. (Show Details)Jun 17 2022, 12:25 AM
This revision is now accepted and ready to land.Jun 20 2022, 4:58 PM
This revision was landed with ongoing or failed builds.Jun 22 2022, 1:15 AM
This revision was automatically updated to reflect the committed changes.