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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
