rsmith added inline comments.
================
Comment at: clang/lib/Parse/ParseDecl.cpp:2290
+ // rounding mode.
+ if (VD->isFileVarDecl() || VD->isConstexpr() ||
+ (!getLangOpts().CPlusPlus && VD->isStaticLocal())) {
----------------
mibintc wrote:
> sepavloff wrote:
> > sepavloff wrote:
> > > rsmith wrote:
> > > > It's far from clear to me that this is correct in C++. In principle,
> > > > for a dynamic initializer, the rounding mode could have been set by an
> > > > earlier initializer.
> > > >
> > > > Perhaps we can make an argument that, due to the permission to
> > > > interleave initializers from different TUs, every dynamic initializer
> > > > must leave the program in the default rounding mode, but I don't think
> > > > even that makes this approach correct, because an initializer could do
> > > > this:
> > > >
> > > > ```
> > > > double d;
> > > > double e = (fesetround(...), d = some calculation,
> > > > fesetround(...default...), d);
> > > > ```
> > > >
> > > > I think we can only do this in C and will need something different for
> > > > C++.
> > > >
> > > > (I think this also has problems in C++ due to constexpr functions: it's
> > > > not the case that all floating point operations that will be evaluated
> > > > as part of the initializer lexically appear within it.)
> > > I changed the code to confine it with C and constexpr only. Hopefully
> > > this would be enough to enable build of SPECS attempted by @mibintc
> > > (https://reviews.llvm.org/D87528#2295015). However in long-term
> > > perspective we should return to this code.
> > >
> > > The intent was to align behavior of C and C++. If an initializer is valid
> > > in C, then it should produce the same code in C++. If the source code like
> > > ```
> > > float appx_coeffs_fp32[3 * 256] = {
> > > SEGMENT_BIAS + 1.4426950216,
> > > …
> > > ```
> > > produces compact table in C mode and huge initialization code in C++, it
> > > would be strange from user viewpoint and would not give any advantage.
> > >
> > > C in C2x presents pretty consistent model, provided that `#pragma STDC
> > > FENV_ROUND FE_DYNAMIC` does not set constant rounding mode. Initializers
> > > for variables with static allocation are always evaluated in constant
> > > rounding mode and user can chose the mode using pragma FENV_ROUND.
> > >
> > > When extending this model to C++ we must solve the problem of dynamic
> > > initialization. It obviously occurs in runtime rounding mode, so changing
> > > between static and dynamic initialization may change semantics. If
> > > however dynamic initialization of global variable occurs in constant
> > > rounding mode (changing FP control modes in initializers without
> > > restoring them is UB), static and dynamic initialization would be
> > > semantically equivalent.
> > >
> > > We cannot apply the same rule to local static variables, as they are
> > > treated differently in C and C++. So the code:
> > > ```
> > > float func_01(float x) {
> > > #pragma STDC FENV_ACCESS ON
> > > static float val = 1.0/3.0;
> > > return val;
> > > }
> > > ```
> > > Would be compiled differently in C and C++.
> > >
> > Additional note:
> >
> > If initialization is dynamic and constant rounding mode is not default, the
> > body of initializer is executed with dynamic rounding mode set to the
> > constant mode. That is, the code:
> > ```
> > #pragma STDC FENV_ROUND FE_UPWARD
> > float var = some_init_expr;
> > ```
> > generates code similar to:
> > ```
> > float var = []()->float {
> > #pragma STDC FENV_ROUND FE_UPWARD
> > return some_init_expr;
> > }
> > ```
> >
> > So initializers of global variable must conform to:
> >
> > 1. If the initialization is dynamic and dynamic rounding mode is changed in
> > the initializer, it must be restored upon finishing the initializer.
> > 2. The initializer is evaluated in constant rounding mode. If the
> > initialization is dynamic, the initializing code is executed in dynamic
> > rounding mode set to the constant rounding mode.
> >
> > These seems enough to provide compatibility with C and the same semantics
> > for static and dynamic initialization.
> @rsmith Serge changed the patch to confine it with C and constexpr only, is
> that adequate to move forward with this patch, and we can return to C++ at
> some point down the road?
This still seems inappropriate for the `constexpr` case -- we shouldn't have
different behavior based on whether an expression appears directly in the
initializer or in a `constexpr` function invoked by that initializer. (It also
violates the C++ standard's recommendation that floating-point evaluation
during translation and at runtime produce the same value.) Please see the
discussion in D87528. Let's continue the conversation there; we shouldn't be
splitting this discussion across two separate review threads.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D88498/new/
https://reviews.llvm.org/D88498
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
