[PATCH] D89360: Treat constant contexts as being in the default rounding mode.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG7e801ca0efa9: Treat constant contexts as being in the 
default rounding mode. (authored by rsmith).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89360/new/

https://reviews.llvm.org/D89360

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/test/Sema/rounding-math.c
  clang/test/SemaCXX/rounding-math.cpp


Index: clang/test/SemaCXX/rounding-math.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/rounding-math.cpp
@@ -0,0 +1,41 @@
+// RUN: %clang_cc1 -triple x86_64-linux -verify=norounding %s
+// RUN: %clang_cc1 -triple x86_64-linux -verify=rounding %s -frounding-math
+// rounding-no-diagnostics
+
+#define fold(x) (__builtin_constant_p(x) ? (x) : (x))
+
+constexpr double a = 1.0 / 3.0;
+
+constexpr int f(int n) { return int(n * (1.0 / 3.0)); }
+
+using T = int[f(3)];
+using T = int[1];
+
+enum Enum { enum_a = f(3) };
+
+struct Bitfield {
+  unsigned int n : 1;
+  unsigned int m : f(3);
+};
+
+void f(Bitfield ) {
+  b.n = int(6 * (1.0 / 3.0)); // norounding-warning {{changes value from 2 to 
0}}
+}
+
+const int k = 3 * (1.0 / 3.0);
+static_assert(k == 1, "");
+
+void g() {
+  // FIXME: Constant-evaluating this initializer is surprising, and violates
+  // the recommended practice in C++ [expr.const]p12:
+  //
+  //   Implementations should provide consistent results of floating-point
+  //   evaluations, irrespective of whether the evaluation is performed during
+  //   translation or during program execution.
+  const int k = 3 * (1.0 / 3.0);
+  static_assert(k == 1, "");
+}
+
+int *h() {
+  return new int[int(-3 * (1.0 / 3.0))]; // norounding-error {{too large}}
+}
Index: clang/test/Sema/rounding-math.c
===
--- /dev/null
+++ clang/test/Sema/rounding-math.c
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -triple x86_64-linux -verify=norounding %s
+// RUN: %clang_cc1 -triple x86_64-linux -std=c17 -verify=rounding-std %s 
-frounding-math
+// RUN: %clang_cc1 -triple x86_64-linux -std=gnu17 -verify=rounding-gnu %s 
-frounding-math
+
+#define fold(x) (__builtin_constant_p(x) ? (x) : (x))
+
+double a = 1.0 / 3.0;
+
+#define f(n) ((n) * (1.0 / 3.0))
+_Static_assert(fold((int)f(3)) == 1, "");
+
+typedef int T[fold((int)f(3))];
+typedef int T[1];
+
+enum Enum { enum_a = (int)f(3) };
+
+struct Bitfield {
+  unsigned int n : 1;
+  unsigned int m : fold((int)f(3));
+};
+
+void bitfield(struct Bitfield *b) {
+  b->n = (int)(6 * (1.0 / 3.0)); // norounding-warning {{changes value from 2 
to 0}}
+}
+
+void vlas() {
+  // Under -frounding-math, this is a VLA.
+  // FIXME: Due to PR44406, in GNU mode we constant-fold the initializer 
resulting in a non-VLA.
+  typedef int vla[(int)(-3 * (1.0 / 3.0))]; // norounding-error {{negative 
size}} rounding-gnu-error {{negative size}}
+  struct X { vla v; }; // rounding-std-error {{fields must have a constant 
size}}
+}
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -2528,6 +2528,11 @@
 /// Check if the given evaluation result is allowed for constant evaluation.
 static bool checkFloatingPointResult(EvalInfo , const Expr *E,
  APFloat::opStatus St) {
+  // In a constant context, assume that any dynamic rounding mode or FP
+  // exception state matches the default floating-point environment.
+  if (Info.InConstantContext)
+return true;
+
   FPOptions FPO = E->getFPFeaturesInEffect(Info.Ctx.getLangOpts());
   if ((St & APFloat::opInexact) &&
   FPO.getRoundingMode() == llvm::RoundingMode::Dynamic) {


Index: clang/test/SemaCXX/rounding-math.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/rounding-math.cpp
@@ -0,0 +1,41 @@
+// RUN: %clang_cc1 -triple x86_64-linux -verify=norounding %s
+// RUN: %clang_cc1 -triple x86_64-linux -verify=rounding %s -frounding-math
+// rounding-no-diagnostics
+
+#define fold(x) (__builtin_constant_p(x) ? (x) : (x))
+
+constexpr double a = 1.0 / 3.0;
+
+constexpr int f(int n) { return int(n * (1.0 / 3.0)); }
+
+using T = int[f(3)];
+using T = int[1];
+
+enum Enum { enum_a = f(3) };
+
+struct Bitfield {
+  unsigned int n : 1;
+  unsigned int m : f(3);
+};
+
+void f(Bitfield ) {
+  b.n = int(6 * (1.0 / 3.0)); // norounding-warning {{changes value from 2 to 0}}
+}
+
+const int k = 3 * (1.0 / 3.0);
+static_assert(k == 1, "");
+
+void g() {
+  // FIXME: Constant-evaluating this initializer is surprising, and violates
+  // the recommended practice in C++ [expr.const]p12:
+  //
+  //   Implementations should provide consistent results of floating-point
+  //   evaluations, irrespective of whether the evaluation is performed during

[PATCH] D89360: Treat constant contexts as being in the default rounding mode.

[John McCall via Phabricator via cfe-commits]

rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.

Okay, thanks.  This LGTM.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89360/new/

https://reviews.llvm.org/D89360

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D89360: Treat constant contexts as being in the default rounding mode.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

In D89360#2329167 , @rjmccall wrote:

> Okay.  I can accept the fairly unfortunate thing with `const int` 
> initializers.  When is `IsConstantContext` set in C, just static initializers?

This affects static initializers, enumerators, bit-widths, array bounds, case 
labels, and indexes in array designators. For code not relying on extensions, 
only static initializers are affected, but as an extension we do permit 
floating-point math in other contexts. I've added some tests for the C side of 
things to demonstrate.

The C tests reveal one issue: block-scope array bounds get constant-folded in 
GNU mode even if they contain floating-point operations. That's PR44406; D89523 
 and D89520  
have a fix for that which should hopefully land soon. I don't think we need to 
block on it.

In D89360#2334038 , @sepavloff wrote:

> Probably there is an issue with code generation. The source:
>
>   constexpr float func_01(float x, float y) {
> return x + y;
>   }
>   
>   float V1 = func_01(1.0F, 0x0.01p0F);
>
> compiled with '-frounding-math' must produce dynamic initializer. It however 
> is evaluated at compile time.

Evaluating it at compile time is in line with the plan from 
https://reviews.llvm.org/D87528#2270676. The C++ rule is that initializers for 
static storage duration variables are first evaluated during translation 
(therefore, per our plan, in the default rounding mode), and only evaluated at 
runtime (and therefore in the runtime rounding mode) if the compile-time 
evaluation fails. This is in line with the C rules; C11 F.8.5 says: "All 
computation for automatic initialization is done (as if) at execution time; 
thus, it is affected by any operative modes and raises floating-point 
exceptions as required by IEC 60559 (provided the state for the FENV_ACCESS 
pragma is ‘‘on’’). All computation for initialization of objects that have 
static or thread storage duration is done (as if) at translation time." C++ 
generalizes this by adding another phase of initialization (at runtime) if the 
translation-time initialization fails, but the translation-time evaluation of 
the initializer of `V1` succeeds so it should be treated as a constant 
initializer.

And just a side note: we're currently blocked by this. C++ builds using 
`-frounding-math` have not worked at all since D87822 
 landed. If we can't come to an agreement on a 
fix very soon, I think we need to revert D87822 
 while we figure it out.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89360/new/

https://reviews.llvm.org/D89360

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D89360: Treat constant contexts as being in the default rounding mode.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith updated this revision to Diff 298724.
rsmith added a comment.

- Add more testcases to demonstrate behavior in C.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89360/new/

https://reviews.llvm.org/D89360

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/test/Sema/rounding-math.c
  clang/test/SemaCXX/rounding-math.cpp


Index: clang/test/SemaCXX/rounding-math.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/rounding-math.cpp
@@ -0,0 +1,41 @@
+// RUN: %clang_cc1 -triple x86_64-linux -verify=norounding %s
+// RUN: %clang_cc1 -triple x86_64-linux -verify=rounding %s -frounding-math
+// rounding-no-diagnostics
+
+#define fold(x) (__builtin_constant_p(x) ? (x) : (x))
+
+constexpr double a = 1.0 / 3.0;
+
+constexpr int f(int n) { return int(n * (1.0 / 3.0)); }
+
+using T = int[f(3)];
+using T = int[1];
+
+enum Enum { enum_a = f(3) };
+
+struct Bitfield {
+  unsigned int n : 1;
+  unsigned int m : f(3);
+};
+
+void f(Bitfield ) {
+  b.n = int(6 * (1.0 / 3.0)); // norounding-warning {{changes value from 2 to 
0}}
+}
+
+const int k = 3 * (1.0 / 3.0);
+static_assert(k == 1, "");
+
+void g() {
+  // FIXME: Constant-evaluating this initializer is surprising, and violates
+  // the recommended practice in C++ [expr.const]p12:
+  //
+  //   Implementations should provide consistent results of floating-point
+  //   evaluations, irrespective of whether the evaluation is performed during
+  //   translation or during program execution.
+  const int k = 3 * (1.0 / 3.0);
+  static_assert(k == 1, "");
+}
+
+int *h() {
+  return new int[int(-3 * (1.0 / 3.0))]; // norounding-error {{too large}}
+}
Index: clang/test/Sema/rounding-math.c
===
--- /dev/null
+++ clang/test/Sema/rounding-math.c
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -triple x86_64-linux -verify=norounding %s
+// RUN: %clang_cc1 -triple x86_64-linux -std=c17 -verify=rounding-std %s 
-frounding-math
+// RUN: %clang_cc1 -triple x86_64-linux -std=gnu17 -verify=rounding-gnu %s 
-frounding-math
+
+#define fold(x) (__builtin_constant_p(x) ? (x) : (x))
+
+double a = 1.0 / 3.0;
+
+#define f(n) ((n) * (1.0 / 3.0))
+_Static_assert(fold((int)f(3)) == 1, "");
+
+typedef int T[fold((int)f(3))];
+typedef int T[1];
+
+enum Enum { enum_a = (int)f(3) };
+
+struct Bitfield {
+  unsigned int n : 1;
+  unsigned int m : fold((int)f(3));
+};
+
+void bitfield(struct Bitfield *b) {
+  b->n = (int)(6 * (1.0 / 3.0)); // norounding-warning {{changes value from 2 
to 0}}
+}
+
+void vlas() {
+  // Under -frounding-math, this is a VLA.
+  // FIXME: Due to PR44406, in GNU mode we constant-fold the initializer 
resulting in a non-VLA.
+  typedef int vla[(int)(-3 * (1.0 / 3.0))]; // norounding-error {{negative 
size}} rounding-gnu-error {{negative size}}
+  struct X { vla v; }; // rounding-std-error {{fields must have a constant 
size}}
+}
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -2528,6 +2528,11 @@
 /// Check if the given evaluation result is allowed for constant evaluation.
 static bool checkFloatingPointResult(EvalInfo , const Expr *E,
  APFloat::opStatus St) {
+  // In a constant context, assume that any dynamic rounding mode or FP
+  // exception state matches the default floating-point environment.
+  if (Info.InConstantContext)
+return true;
+
   FPOptions FPO = E->getFPFeaturesInEffect(Info.Ctx.getLangOpts());
   if ((St & APFloat::opInexact) &&
   FPO.getRoundingMode() == llvm::RoundingMode::Dynamic) {


Index: clang/test/SemaCXX/rounding-math.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/rounding-math.cpp
@@ -0,0 +1,41 @@
+// RUN: %clang_cc1 -triple x86_64-linux -verify=norounding %s
+// RUN: %clang_cc1 -triple x86_64-linux -verify=rounding %s -frounding-math
+// rounding-no-diagnostics
+
+#define fold(x) (__builtin_constant_p(x) ? (x) : (x))
+
+constexpr double a = 1.0 / 3.0;
+
+constexpr int f(int n) { return int(n * (1.0 / 3.0)); }
+
+using T = int[f(3)];
+using T = int[1];
+
+enum Enum { enum_a = f(3) };
+
+struct Bitfield {
+  unsigned int n : 1;
+  unsigned int m : f(3);
+};
+
+void f(Bitfield ) {
+  b.n = int(6 * (1.0 / 3.0)); // norounding-warning {{changes value from 2 to 0}}
+}
+
+const int k = 3 * (1.0 / 3.0);
+static_assert(k == 1, "");
+
+void g() {
+  // FIXME: Constant-evaluating this initializer is surprising, and violates
+  // the recommended practice in C++ [expr.const]p12:
+  //
+  //   Implementations should provide consistent results of floating-point
+  //   evaluations, irrespective of whether the evaluation is performed during
+  //   translation or during program execution.
+  const int k = 3 * (1.0 / 3.0);
+  static_assert(k == 1, "");
+}
+
+int 

[PATCH] D89360: Treat constant contexts as being in the default rounding mode.

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff added a comment.

Probably there is an issue with code generation. The source:

  constexpr float func_01(float x, float y) {
return x + y;
  }
  
  float V1 = func_01(1.0F, 0x0.01p0F);

compiled with '-frounding-math' must produce dynamic initializer. It however is 
evaluated at compile time.




Comment at: clang/test/SemaCXX/rounding-math.cpp:5
+
+#define fold(x) (__builtin_constant_p(x) ? (x) : (x))
+

It is not used.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89360/new/

https://reviews.llvm.org/D89360

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D89360: Treat constant contexts as being in the default rounding mode.

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff added a comment.

As C++ case is much more complicated and we need a ruleset based on essential 
properties rather than implementation viewpoint, I am convinced that the 
solution based on manifestly constant-evaluated property is the best. 
Thank you for the discussion!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89360/new/

https://reviews.llvm.org/D89360

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D89360: Treat constant contexts as being in the default rounding mode.

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff added a comment.

In D89360#2330522 , @rsmith wrote:

> In D89360#2329856 , @sepavloff wrote:
>
>> I would propose to consider solution in D88498 
>> . It tries to fix the real reason of the 
>> malfunction - using dynamic rounding mode for evaluation of global variable 
>> initializers.
>
> That is not the real reason for the malfunction. If you narrowly look at C, 
> you can convince yourself otherwise, but that's only because global variable 
> initializers are the only place where we need to evaluate floating-point 
> constant expressions in C. In C++, this problem affects all contexts where 
> constant evaluation might happen (array bounds, bit-field widths, and a whole 
> host of others).

You are right, the solution in D88498  was 
more a workaround, as it was focused on initializers only. The updated version 
applies dynamic rounding to function bodies only, I hope it could solve the 
problem of other contexts.




Comment at: clang/test/SemaCXX/rounding-math.cpp:9
+
+constexpr int f(int n) { return int(n * (1.0 / 3.0)); }
+

rsmith wrote:
> rsmith wrote:
> > sepavloff wrote:
> > > This code requires additional solution. The function body is built using 
> > > dynamic rounding mode, which breaks its constexprness. We can avoid this 
> > > kind of errors if we assume that body of a constexpr function is parsed 
> > > using constant rounding mode (in this example it is the default mode). It 
> > > makes parsing constexpr function different from non-constexpr ones, but 
> > > enables convenient use:
> > > ```
> > > constexpr int add(float x, float y) { return x + y; }
> > > 
> > > #pragma STDC FENV_ROUND FE_UPWARD
> > > int a2 = add(2.0F, 0x1.02p0F);
> > > 
> > > #pragma STDC FENV_ROUND FE_DOWNWARD
> > > int a3 = add(2.0F, 0x1.02p0F);
> > > ```
> > > If calls of constexpr functions are processes with FP options acting in 
> > > the call site, a call to constexpr function becomes equivalent to 
> > > execution of statements of its body.
> > I don't understand what you mean by "breaks its constexprness". Using a 
> > dynamic rounding mode for the body of the function is exactly what we want 
> > here. When the function is called in a manifestly constant evaluated 
> > context, we should use the default rounding mode, and when it's called at 
> > runtime, we use the appropriate dynamic rounding mode; if we try to 
> > constant evaluate it outside of a manifestly constant evaluated constant, 
> > we deem it non-constant.
> > 
> > When `constexpr` function is called at runtime, it's a completely normal 
> > function with normal semantics. It would be wrong to use the default 
> > rounding mode when parsing its body.
> To be clear: in your example with `add`, both `a2` and `a3` should be 
> initialized to the same value, which should be rounded with the default 
> rounding mode. Per ISO18661, `FENV_ROUND` only affects operations in its 
> lexical scope, not functions called within those operations, and per the 
> regular C++ rules, `constexpr` functions behave like regular functions, not 
> like macros.
I incorrectly identified the reason why some call was rejected in constant 
evaluated context. Please ignore this statement. I am sorry.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89360/new/

https://reviews.llvm.org/D89360

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D89360: Treat constant contexts as being in the default rounding mode.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.



Comment at: clang/test/SemaCXX/rounding-math.cpp:9
+
+constexpr int f(int n) { return int(n * (1.0 / 3.0)); }
+

rsmith wrote:
> sepavloff wrote:
> > This code requires additional solution. The function body is built using 
> > dynamic rounding mode, which breaks its constexprness. We can avoid this 
> > kind of errors if we assume that body of a constexpr function is parsed 
> > using constant rounding mode (in this example it is the default mode). It 
> > makes parsing constexpr function different from non-constexpr ones, but 
> > enables convenient use:
> > ```
> > constexpr int add(float x, float y) { return x + y; }
> > 
> > #pragma STDC FENV_ROUND FE_UPWARD
> > int a2 = add(2.0F, 0x1.02p0F);
> > 
> > #pragma STDC FENV_ROUND FE_DOWNWARD
> > int a3 = add(2.0F, 0x1.02p0F);
> > ```
> > If calls of constexpr functions are processes with FP options acting in the 
> > call site, a call to constexpr function becomes equivalent to execution of 
> > statements of its body.
> I don't understand what you mean by "breaks its constexprness". Using a 
> dynamic rounding mode for the body of the function is exactly what we want 
> here. When the function is called in a manifestly constant evaluated context, 
> we should use the default rounding mode, and when it's called at runtime, we 
> use the appropriate dynamic rounding mode; if we try to constant evaluate it 
> outside of a manifestly constant evaluated constant, we deem it non-constant.
> 
> When `constexpr` function is called at runtime, it's a completely normal 
> function with normal semantics. It would be wrong to use the default rounding 
> mode when parsing its body.
To be clear: in your example with `add`, both `a2` and `a3` should be 
initialized to the same value, which should be rounded with the default 
rounding mode. Per ISO18661, `FENV_ROUND` only affects operations in its 
lexical scope, not functions called within those operations, and per the 
regular C++ rules, `constexpr` functions behave like regular functions, not 
like macros.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89360/new/

https://reviews.llvm.org/D89360

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D89360: Treat constant contexts as being in the default rounding mode.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

In D89360#2329856 , @sepavloff wrote:

> I would propose to consider solution in D88498 
> . It tries to fix the real reason of the 
> malfunction - using dynamic rounding mode for evaluation of global variable 
> initializers.

That is not the real reason for the malfunction. If you narrowly look at C, you 
can convince yourself otherwise, but that's only because global variable 
initializers are the only place where we need to evaluate floating-point 
constant expressions in C. In C++, this problem affects all contexts where 
constant evaluation might happen (array bounds, bit-field widths, and a whole 
host of others).

D88498 's approach also can't correctly handle 
`constexpr` functions, for which we want different behavior depending on 
whether the *caller* is a manifestly constant evaluated context.




Comment at: clang/test/SemaCXX/rounding-math.cpp:9
+
+constexpr int f(int n) { return int(n * (1.0 / 3.0)); }
+

sepavloff wrote:
> This code requires additional solution. The function body is built using 
> dynamic rounding mode, which breaks its constexprness. We can avoid this kind 
> of errors if we assume that body of a constexpr function is parsed using 
> constant rounding mode (in this example it is the default mode). It makes 
> parsing constexpr function different from non-constexpr ones, but enables 
> convenient use:
> ```
> constexpr int add(float x, float y) { return x + y; }
> 
> #pragma STDC FENV_ROUND FE_UPWARD
> int a2 = add(2.0F, 0x1.02p0F);
> 
> #pragma STDC FENV_ROUND FE_DOWNWARD
> int a3 = add(2.0F, 0x1.02p0F);
> ```
> If calls of constexpr functions are processes with FP options acting in the 
> call site, a call to constexpr function becomes equivalent to execution of 
> statements of its body.
I don't understand what you mean by "breaks its constexprness". Using a dynamic 
rounding mode for the body of the function is exactly what we want here. When 
the function is called in a manifestly constant evaluated context, we should 
use the default rounding mode, and when it's called at runtime, we use the 
appropriate dynamic rounding mode; if we try to constant evaluate it outside of 
a manifestly constant evaluated constant, we deem it non-constant.

When `constexpr` function is called at runtime, it's a completely normal 
function with normal semantics. It would be wrong to use the default rounding 
mode when parsing its body.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89360/new/

https://reviews.llvm.org/D89360

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D89360: Treat constant contexts as being in the default rounding mode.

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff added a comment.

I would propose to consider solution in D88498 
. It tries to fix the real reason of the 
malfunction - using dynamic rounding mode for evaluation of global variable 
initializers. With the workaround for constexpr functions it allows to compile 
the test attached and still correctly detect runtime nature of local variable 
initializers.




Comment at: clang/lib/AST/ExprConstant.cpp:2533-2534
+  // exception state matches the default floating-point environment.
+  if (Info.InConstantContext)
+return true;
+

It turns off the check made by this function. In the case of global variable 
initializer it fixes the error (using dynamic rounding mode instead of default) 
but for local variable initializer it creates a new error. Constant evaluator 
cannot detect that the initializer in the code:
```
void g() {
  const int k = 3 * (1.0 / 3.0);
  ...
}
```
is not a constant expression.



Comment at: clang/test/SemaCXX/rounding-math.cpp:9
+
+constexpr int f(int n) { return int(n * (1.0 / 3.0)); }
+

This code requires additional solution. The function body is built using 
dynamic rounding mode, which breaks its constexprness. We can avoid this kind 
of errors if we assume that body of a constexpr function is parsed using 
constant rounding mode (in this example it is the default mode). It makes 
parsing constexpr function different from non-constexpr ones, but enables 
convenient use:
```
constexpr int add(float x, float y) { return x + y; }

#pragma STDC FENV_ROUND FE_UPWARD
int a2 = add(2.0F, 0x1.02p0F);

#pragma STDC FENV_ROUND FE_DOWNWARD
int a3 = add(2.0F, 0x1.02p0F);
```
If calls of constexpr functions are processes with FP options acting in the 
call site, a call to constexpr function becomes equivalent to execution of 
statements of its body.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89360/new/

https://reviews.llvm.org/D89360

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D89360: Treat constant contexts as being in the default rounding mode.

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Okay.  I can accept the fairly unfortunate thing with `const int` initializers. 
 When is `IsConstantContext` set in C, just static initializers?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89360/new/

https://reviews.llvm.org/D89360

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D89360: Treat constant contexts as being in the default rounding mode.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith created this revision.
rsmith added reviewers: rjmccall, mibintc, sepavloff.
Herald added a project: clang.
rsmith requested review of this revision.

This addresses a regression where pretty much all C++ compilations using
-frounding-math now fail, due to rounding being performed in constexpr
function definitions in the standard library.

This follows the "manifestly constant evaluated" approach described in
https://reviews.llvm.org/D87528#2270676 -- evaluations that are required
to succeed at compile time are permitted even in regions with dynamic
rounding modes, as are (unfortunately) the evaluation of the
initializers of local variables of const integral types.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D89360

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/test/SemaCXX/rounding-math.cpp


Index: clang/test/SemaCXX/rounding-math.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/rounding-math.cpp
@@ -0,0 +1,36 @@
+// RUN: %clang_cc1 -triple x86_64-linux -verify=norounding %s
+// RUN: %clang_cc1 -triple x86_64-linux -verify=rounding %s -frounding-math
+// rounding-no-diagnostics
+
+#define fold(x) (__builtin_constant_p(x) ? (x) : (x))
+
+constexpr double a = 1.0 / 3.0;
+
+constexpr int f(int n) { return int(n * (1.0 / 3.0)); }
+
+using T = int[f(3)];
+using T = int[1];
+
+struct Bitfield { unsigned int n : 1; };
+
+void f(Bitfield ) {
+  b.n = int(6 * (1.0 / 3.0)); // norounding-warning {{changes value from 2 to 
0}}
+}
+
+const int k = 3 * (1.0 / 3.0);
+static_assert(k == 1, "");
+
+void g() {
+  // FIXME: Constant-evaluating this initializer is surprising, and violates
+  // the recommended practice in C++ [expr.const]p12:
+  //
+  //   Implementations should provide consistent results of floating-point
+  //   evaluations, irrespective of whether the evaluation is performed during
+  //   translation or during program execution.
+  const int k = 3 * (1.0 / 3.0);
+  static_assert(k == 1, "");
+}
+
+int *h() {
+  return new int[int(-3 * (1.0 / 3.0))]; // norounding-error {{too large}}
+}
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -2528,6 +2528,11 @@
 /// Check if the given evaluation result is allowed for constant evaluation.
 static bool checkFloatingPointResult(EvalInfo , const Expr *E,
  APFloat::opStatus St) {
+  // In a constant context, assume that any dynamic rounding mode or FP
+  // exception state matches the default floating-point environment.
+  if (Info.InConstantContext)
+return true;
+
   FPOptions FPO = E->getFPFeaturesInEffect(Info.Ctx.getLangOpts());
   if ((St & APFloat::opInexact) &&
   FPO.getRoundingMode() == llvm::RoundingMode::Dynamic) {


Index: clang/test/SemaCXX/rounding-math.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/rounding-math.cpp
@@ -0,0 +1,36 @@
+// RUN: %clang_cc1 -triple x86_64-linux -verify=norounding %s
+// RUN: %clang_cc1 -triple x86_64-linux -verify=rounding %s -frounding-math
+// rounding-no-diagnostics
+
+#define fold(x) (__builtin_constant_p(x) ? (x) : (x))
+
+constexpr double a = 1.0 / 3.0;
+
+constexpr int f(int n) { return int(n * (1.0 / 3.0)); }
+
+using T = int[f(3)];
+using T = int[1];
+
+struct Bitfield { unsigned int n : 1; };
+
+void f(Bitfield ) {
+  b.n = int(6 * (1.0 / 3.0)); // norounding-warning {{changes value from 2 to 0}}
+}
+
+const int k = 3 * (1.0 / 3.0);
+static_assert(k == 1, "");
+
+void g() {
+  // FIXME: Constant-evaluating this initializer is surprising, and violates
+  // the recommended practice in C++ [expr.const]p12:
+  //
+  //   Implementations should provide consistent results of floating-point
+  //   evaluations, irrespective of whether the evaluation is performed during
+  //   translation or during program execution.
+  const int k = 3 * (1.0 / 3.0);
+  static_assert(k == 1, "");
+}
+
+int *h() {
+  return new int[int(-3 * (1.0 / 3.0))]; // norounding-error {{too large}}
+}
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -2528,6 +2528,11 @@
 /// Check if the given evaluation result is allowed for constant evaluation.
 static bool checkFloatingPointResult(EvalInfo , const Expr *E,
  APFloat::opStatus St) {
+  // In a constant context, assume that any dynamic rounding mode or FP
+  // exception state matches the default floating-point environment.
+  if (Info.InConstantContext)
+return true;
+
   FPOptions FPO = E->getFPFeaturesInEffect(Info.Ctx.getLangOpts());
   if ((St & APFloat::opInexact) &&
   FPO.getRoundingMode() == llvm::RoundingMode::Dynamic) {
___
cfe-commits mailing