[PATCH] D126364: Fix interaction of pragma FENV_ACCESS with other pragmas

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff added a comment.

In D126364#3537964 <https://reviews.llvm.org/D126364#3537964>, @efriedma wrote:

> 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

2. Strict exception mode:

  	#pragma STDC FENV_ACCESS ON
  	#pragma clang fp exception(strict)

3. MayTrap exception mode:

  	#pragma STDC FENV_ACCESS ON
  	#pragma clang fp exception(maytrap)

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126364