Page MenuHomePhabricator

Represent FP options in AST by special Expression node
Needs ReviewPublic

Authored by sepavloff on Apr 6 2020, 6:03 AM.

Details

Summary

The patch D76599 proposed a new statement node to represent pragmas that
modifies floating point environment. These nodes are used to implement
FP environment tracking and control in a function. Such node modify FP
environment in enclosing compound statement thus organizing FP
environment in hierarchical manner.

That solution however cannot be applied to initializers of global
variables and some other objects. They represent expressions outside
function bodies and thus cannot be embedded into compound statements.

Expression node introduced here (FPEnvironmentExpr) is to represent
FP environment in such cases. It contains a field of type FPOptions,
which specifies FP environment in which subexpression of
FPEnvironmentExpr should be executed. The patch implements FP
environment in the following cases:

  • Initializers of global variables, like:

    float v1 = 3,1415926535;
  • Inline initializers of fields:

    struct C1 { float f1 = 1,4142135623; };
  • Arguments of base constructors in constructors:

    struct C3 : public C2 { C3(double x, double y) : C2(x + y) {} };
  • Default function arguments:

    void func_01(float x = 1.1);

In contrast to the solution for compound statements in D76599, this
solution does not keep reference to the source code construct that set
the FP environment, namely the file scope pragma. It may cause problems
for AST consumers like source analyzers.

Diff Detail

Event Timeline

sepavloff created this revision.Apr 6 2020, 6:03 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 6 2020, 6:03 AM
Herald added a subscriber: martong. · View Herald Transcript
martong removed a subscriber: martong.Apr 6 2020, 6:45 AM

The goal here seems to be to avoid the need to store pragma state in operator expressions. As I mentioned in another review, I'm not sure how directly interesting that goal is if we can avoid memory overhead in the common case. Storing pragma state in operators certainly has a substantial complexity cost, but forcing every AST client to track the currently-active pragma state also has a cost.

The solution in D76384 (Move FPFeatures from BinaryOperator bitfields to Trailing storage) causes several concerns.

  1. It requires substantial code changes. The patch in 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.
  1. This solution means some loss of source information. Pragma is not represented in AST and some source analysis tools probably may suffer from it.
  1. 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.
  1. 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.

  1. 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 solution in D76384 (Move FPFeatures from BinaryOperator bitfields to Trailing storage) causes several concerns.

  1. It requires substantial code changes. The patch in 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.

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

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

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

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

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.

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

! In D77545#1973765, @sepavloff wrote:

! In D77545#1973169, @rjmccall wrote:
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

Okay, so you're proposing that we maintain global state in the ASTContext for what pragmas are in effect, managed by RAII objects, rather than passing the current pragma state in as an argument. I agree that this is relatively straightforward for clients to support if they're just performing a recursive walk. However, it is extremely susceptible to bugs where the current context is incorrectly applied to expressions written in a different context.

For example, in this code:

const float global = 2.22 + 3.33;

#pragma STDV FENV_ROUND <direction>
float getGlobal() { return global; }

The code-generation of getGlobal will attempt to constant-evaluate the initializer of global so that it can just return a constant instead of emitting a load from a global. It is important for correctness that this constant-evaluation occur in the default FP environment, not the active FP environment at the point of use. Presumably you are not proposing that global's initializer is a PragmaExpr, since it's in the default environment. Perhaps we can recognize by the *absence* of a PragmaExpr that it's meant to use the default environment? It's not clear at what level we would trigger this check, however. Is it now semantically important that we call a method like VarDecl::evaluateValue() rather than Expr::EvaluateAsFloat(...)? What happens with variables in local scope? A local variable could be from a scope outside the current pragma, so presumably we need to introduce PragmaExprs on the initializers of all variables within pragmas.

This sort of problem is why having the pragma state be an argument rather than global state is so much more attractive. And making it an argument is much more disruptive for clients, who must now do their own tracking, and recreate the state appropriately when calling outside the context.

Storing the pragma state in the expressions avoids these problems much more cleanly.

For example, in this code:

const float global = 2.22 + 3.33;

#pragma STDV FENV_ROUND <direction>
float getGlobal() { return global; }

The code-generation of getGlobal will attempt to constant-evaluate the initializer of global so that it can just return a constant instead of emitting a load from a global. It is important for correctness that this constant-evaluation occur in the default FP environment, not the active FP environment at the point of use. Presumably you are not proposing that global's initializer is a PragmaExpr, since it's in the default environment. Perhaps we can recognize by the *absence* of a PragmaExpr that it's meant to use the default environment? It's not clear at what level we would trigger this check, however. Is it now semantically important that we call a method like VarDecl::evaluateValue() rather than Expr::EvaluateAsFloat(...)? What happens with variables in local scope? A local variable could be from a scope outside the current pragma, so presumably we need to introduce PragmaExprs on the initializers of all variables within pragmas.

As you correctly noted, putting PragmaExpr around initializer expression even in default environment solves all these problems. This solution may be improved to put PragmaExpr only when the expression may be dependent on FP environment.

This sort of problem is why having the pragma state be an argument rather than global state is so much more attractive. And making it an argument is much more disruptive for clients, who must now do their own tracking, and recreate the state appropriately when calling outside the context.

Storing the pragma state in the expressions avoids these problems much more cleanly.

I see at least two issues with such solution:

  1. There are cases when part of AST should be interpreted in other context than it was created. The example was presented earlier, this is constexpr function evaluation:
        #pragma STDC FENV_ACCESS ON
	constexpr float func(float x, float y) { return x + y;	}
	
	#pragma STDC FENV_ROUND FE_DOWNWARD
	float V1 = func(1.11, 2.22);
	
	#pragma STDC FENV_ROUND FE_UPWARD
	float V2 = func(1.11, 2.22);

If FP environment is encoded in AST nodes, then the body of func should use FE_DYNAMIC as rounding mode. During evaluation such nodes this value should be replaced by the rounding mode taken from current context. Constant evaluator anyway should use global state and RoundingMode::Dynamic must be interpreted specifically.

  1. This solution requires huge changes to AST node implementation, as demonstrated by D76384. And what will we do with CallExpr and CastExpr, which already have TrailingObjects? How this solution can be extended? For instance, we may want to make ExprWithCleanups sensitive to FP environment so that it can restore previous state. We have to start with redesign of ExprWithCleanups so that it contains FPOptions. As this node already contains TrailingObjects it is not a trivial task. Any node that designates an operation that can depend on FP state should be reworked first.

In contrast, using special nodes to represent global state make this task transparently. No changes are required to existing AST nodes, only AST consumers that are aware of FP state need to be updated. This way looks more robust due to orthogonal and less invasive implementation.

For example, in this code:

const float global = 2.22 + 3.33;

#pragma STDV FENV_ROUND <direction>
float getGlobal() { return global; }

The code-generation of getGlobal will attempt to constant-evaluate the initializer of global so that it can just return a constant instead of emitting a load from a global. It is important for correctness that this constant-evaluation occur in the default FP environment, not the active FP environment at the point of use. Presumably you are not proposing that global's initializer is a PragmaExpr, since it's in the default environment. Perhaps we can recognize by the *absence* of a PragmaExpr that it's meant to use the default environment? It's not clear at what level we would trigger this check, however. Is it now semantically important that we call a method like VarDecl::evaluateValue() rather than Expr::EvaluateAsFloat(...)? What happens with variables in local scope? A local variable could be from a scope outside the current pragma, so presumably we need to introduce PragmaExprs on the initializers of all variables within pragmas.

As you correctly noted, putting PragmaExpr around initializer expression even in default environment solves all these problems. This solution may be improved to put PragmaExpr only when the expression may be dependent on FP environment.

As someone who's very familiar with how Clang and its various tools generally use the constant evaluator, I am telling you that reasoning by implication from the presence or absence of a direct PragmaExpr on an initializer is going to involve a lot of extremely subtle interactions and a very long tail of bugs. Since those bugs are going to be localized to this one extremely obscure feature, I do not think that is a good idea.

This sort of problem is why having the pragma state be an argument rather than global state is so much more attractive. And making it an argument is much more disruptive for clients, who must now do their own tracking, and recreate the state appropriately when calling outside the context.

Storing the pragma state in the expressions avoids these problems much more cleanly.

I see at least two issues with such solution:

  1. There are cases when part of AST should be interpreted in other context than it was created. The example was presented earlier, this is constexpr function evaluation:

    ` #pragma STDC FENV_ACCESS ON constexpr float func(float x, float y) { return x + y; } #pragma STDC FENV_ROUND FE_DOWNWARD float V1 = func(1.11, 2.22); #pragma STDC FENV_ROUND FE_UPWARD float V2 = func(1.11, 2.22); ` If FP environment is encoded in AST nodes, then the body of func should use FE_DYNAMIC as rounding mode. During evaluation such nodes this value should be replaced by the rounding mode taken from current context. Constant evaluator anyway should use global state and RoundingMode::Dynamic must be interpreted specifically.

(1) It is not a good idea to use actual global state to model machine state within a constant evaluation because information is extremely prone to leaking in and out.
(2) We are specifically *required* by the standard to ignore FENV_ACCESS ON pragmas in mandatory constant evaluation.

  1. This solution requires huge changes to AST node implementation, as demonstrated by D76384.

That patch has actually gotten much smaller.

And what will we do with CallExpr and CastExpr, which already have TrailingObjects?

It's actually quite easy to add more trailing fields to an AST node that already has them. Most of the boilerplate in D76384 is from switching new-expessions to calls to static factories, but you don't need to do that when a node is already using factories.

For instance, we may want to make ExprWithCleanups sensitive to FP environment so that it can restore previous state. We have to start with redesign of ExprWithCleanups so that it contains FPOptions. As this node already contains TrailingObjects it is not a trivial task.

This representation does not model these things as explicit FP environment state changes that would need to be undone on an exception. But also, adding more trailing objects to a node is not a significant burden.