sepavloff added a comment.
In D77545#1974509 <https://reviews.llvm.org/D77545#1974509>, @rjmccall wrote:
> 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 `PragmaExpr`s 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.
2. This solution requires huge changes to AST node implementation, as
demonstrated by D76384 <https://reviews.llvm.org/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.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D77545/new/
https://reviews.llvm.org/D77545
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
