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

John McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Apr 9 15:48:44 PDT 2020


rjmccall added a comment.

In D77545#1972344 <https://reviews.llvm.org/D77545#1972344>, @sepavloff wrote:

> The solution in D76384 <https://reviews.llvm.org/D76384> (Move FPFeatures from BinaryOperator bitfields to Trailing storage) causes several concerns.
>
> 1. It requires substantial code changes. The patch in D76384 <https://reviews.llvm.org/D76384> is already large and we need to do similar work for UnaryOperator, CallExpr, CXXOperatorCallExpr, CastExpr and probably some others. In contrast, using pragma nodes does not require changes to existing nodes.


Pragma nodes don't require AST changes to existing nodes, but they require large changes to a number of clients, 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.

> 2. This solution means some loss of source information. Pragma is not represented in AST and some source analysis tools probably may suffer from it.

We can reflect the pragma in the AST even if it isn't necessary for correctness.

> 3. In some cases pragma anyway must be present in AST. For instance, it is useful to have a pragma that sets rounding mode, without need of `fesetround`. Such pragma would make codegen to emit some code, it is reasonable to do that during treatment of corresponding statement node rather than of CompoundStmt.

Since such a pragma would only affect the code in the function, it would not admit an implementation that just calls `fesetround`, because the original state would need to be restored on any function calls.

> 4. Replication of FP environment into separate operation nodes actually change semantics. IEEE-754 considers FP environment as global state and this viewpoint is consistent with hardware viewpoint on many platforms. The replication make the environment a sum of operation properties. This change manifests itself in some cases. For instance, the following code: ``` 	const float C1 = 3,1415926535; 	const float C2 = 1,4142135623; 	 	#pragma STDC FENV_ACCESS ON 	constexpr float func(float x, float y) { 	    return x + y; 	} 	 	#pragma STDC FENV_ROUND FE_DOWNWARD 	float V1 = func(C1, C2); 	 	#pragma STDC FENV_ROUND FE_UPWARD 	float V2 = func(C1, C2); ```	 If FP environment is replicated into every operation in the body of `func`, the operations will get "round.dynamic" as rounding mode argument. Such nodes cannot be evaluated in compile time. But if FP environment is global state, constant evaluator could just set it before evaluation. Behavior of constexpr functions in this case produce the same results as for non-constexpr variants.

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.

> 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 concern is that we don't want the constant evaluator to become sensitive to arbitrary mutable state in the larger compiler.


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