[PATCH] D77545: Represent FP options in AST by special Expression node

Serge Pavlov via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Apr 9 21:45:31 PDT 2020


sepavloff added a comment.

In D77545#1973169 <https://reviews.llvm.org/D77545#1973169>, @rjmccall wrote:

> Pragma nodes don't require AST changes to existing nodes, but they require large changes to a number of clients,


If a client is unaware of peculiarities of floating point operations, it can safely ignore existence of FP environment. For example, ASTMatcher is such a client. No change are required in this case.

Some clients, like ASTDumper require modification to support new nodes. The changes in this case are not larger than typical change required to support a new AST node.

There are clients that can benefit from knowledge about FP environment, probably StaticAnalyzer is such. The relevant support requires almost the same efforts in any case.

Some clients, like Codegen and Constant Evaluator must take into account  FP environment. They must be updated properly as now we have no such support. It is questionable which approach requires more efforts, I think both are similar in complexity.

The changes to clients in the case of 'global state' approach are not large than in the case of 'trailing objects'.

> all of which will just do the wrong thing if they don't preserve and pass down the right information.  It's extremely error-prone, and I don't see a way to make it not error-prone.

Actually support of FP environment is very similar to scope mechanism. It is changed *only* at compound statement entry or exit. We already have many RAII objects that facilitate this maintenance

> C specifies that constant evaluation contexts do not honor `FENV_ACCESS`.  I'm not sure it's specified anywhere for C++, but it presumably applies equally to at least explicit `constexpr` contexts.

They must honor `FENV_ROUND`:

  n2454, F.8.2
  During translation, constant rounding direction modes (7.6.2) are in effect where specified.
  
  n2454, 7.6.2p2
  The FENV_ROUND pragma provides a means to specify a constant rounding direction for floating point
  operations for standard floating types within a translation unit or compound statement.

If initialization with call to constexpr function should be equivalent to similar constant expression, like in:

  #pragma STDC FENV_ROUND <direction>
  constexpr float add(float x, float y) { return x+y; }
  float N1 = 2.22 + 3.33;
  float N2 = add(2.22, 3.33);

compiler must evaluate constexpr function in the current FP environment.

>> 5. Rounding mode can be turned into local property, but exception status anyway would require global state in constant evaluator. A function like: ``` 	constexpr float func(float x, float y) { 	    float z = x + y; 	    if (fetestexcept(FE_OVERFLOW)) 	        z = 0; 	    return z; 	} ```	 can be evaluated in compile time but exceptions raised by operations need to be kept in some global state. So maintaining this global state anyway required for constant evaluator, no matter if it contains rounding mode or not.
> 
> The constant evaluator can certainly model state within its evaluation, but that state is local to an evaluation.  You would not constant-evaluate one call that sets an FP exception and then expect a later constant-evaluation to be able to detect that.

The example with exception tracking was not good. I wanted only to demonstrate that constant evaluator inevitably should maintain global state if we want it to calculate exception state. It must be convenient, if an expression overflows, do one thing, if not do another. Using FP environment as a "hidden" argument to constexpr function is of course impossible.

> The concern is that we don't want the constant evaluator to become sensitive to arbitrary mutable state in the larger compiler.

Not to arbitrary state, but to some part of FP state, namely FP control modes, which now is only rounding mode, set by `pragma STDC FENV_ROUND`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77545





More information about the cfe-commits mailing list