[PATCH] D126364: Fix interaction of pragma FENV_ACCESS with other pragmas

[Serge Pavlov 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 rG706e89db97d3: Fix interaction of pragma FENV_ACCESS with 
other pragmas (authored by sepavloff).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126364

Files:
  clang/include/clang/Basic/FPOptions.def
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Driver/Options.td
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ExprConstant.cpp
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Parse/ParsePragma.cpp
  clang/lib/Sema/SemaAttr.cpp
  clang/test/AST/ast-dump-fpfeatures.cpp
  clang/test/CodeGen/pragma-fenv_access.c

Index: clang/test/CodeGen/pragma-fenv_access.c
===
--- clang/test/CodeGen/pragma-fenv_access.c
+++ clang/test/CodeGen/pragma-fenv_access.c
@@ -1,5 +1,15 @@
-// RUN: %clang_cc1 -fexperimental-strict-floating-point -ffp-exception-behavior=strict -triple %itanium_abi_triple -emit-llvm %s -o - | FileCheck %s
-// RUN: %clang_cc1 -fexperimental-strict-floating-point -ffp-exception-behavior=strict -triple %itanium_abi_triple -emit-llvm %s -o - -fms-extensions -DMS | FileCheck %s
+// RUN: %clang_cc1 -fexperimental-strict-floating-point -ffp-exception-behavior=strict -triple %itanium_abi_triple -emit-llvm %s -o - | FileCheck --check-prefixes=CHECK,STRICT %s
+// RUN: %clang_cc1 -fexperimental-strict-floating-point -ffp-exception-behavior=strict -triple %itanium_abi_triple -emit-llvm %s -o - -fms-extensions -DMS | FileCheck --check-prefixes=CHECK,STRICT %s
+// RUN: %clang_cc1 -fexperimental-strict-floating-point -triple %itanium_abi_triple -emit-llvm %s -o - | FileCheck --check-prefixes=CHECK,DEFAULT %s
+
+
+float func_00(float x, float y) {
+  return x + y;
+}
+// CHECK-LABEL: @func_00
+// STRICT: call float @llvm.experimental.constrained.fadd.f32(float {{.*}}, float {{.*}}, metadata !"round.tonearest", metadata !"fpexcept.strict")
+// DEFAULT: fadd float
+
 
 #ifdef MS
 #pragma fenv_access (on)
@@ -20,7 +30,7 @@
   return x + y;
 }
 // CHECK-LABEL: @func_02
-// CHECK: call float @llvm.experimental.constrained.fadd.f32(float {{.*}}, float {{.*}}, metadata !"round.dynamic", metadata !"fpexcept.ignore")
+// CHECK: call float @llvm.experimental.constrained.fadd.f32(float {{.*}}, float {{.*}}, metadata !"round.tonearest", metadata !"fpexcept.ignore")
 
 
 float func_03(float x, float y) {
@@ -41,7 +51,16 @@
   return x + y;
 }
 // CHECK-LABEL: @func_04
-// CHECK: call float @llvm.experimental.constrained.fadd.f32(float {{.*}}, float {{.*}}, metadata !"round.dynamic", metadata !"fpexcept.ignore")
+// STRICT: call float @llvm.experimental.constrained.fadd.f32(float {{.*}}, float {{.*}}, metadata !"round.tonearest", metadata !"fpexcept.ignore")
+// DEFAULT: fadd float
+
+
+float func_04a(float x, float y) {
+  #pragma float_control(except, on)
+  return x + y;
+}
+// CHECK-LABEL: @func_04a
+// CHECK: call float @llvm.experimental.constrained.fadd.f32(float {{.*}}, float {{.*}}, metadata !"round.tonearest", metadata !"fpexcept.strict")
 
 
 float func_05(float x, float y) {
@@ -57,18 +76,151 @@
   return x + y;
 }
 // CHECK-LABEL: @func_06
-// CHECK: call float @llvm.experimental.constrained.fadd.f32(float {{.*}}, float {{.*}}, metadata !"round.dynamic", metadata !"fpexcept.ignore")
+// STRICT: call float @llvm.experimental.constrained.fadd.f32(float {{.*}}, float {{.*}}, metadata !"round.tonearest", metadata !"fpexcept.ignore")
+// DEFAULT: fadd float
 
 
 float func_07(float x, float y) {
   x -= y;
   if (x) {
 #pragma STDC FENV_ACCESS ON
-y *= 2;
+y *= 2.0F;
   }
-  return y + 4;
+  return y + 4.0F;
 }
 // CHECK-LABEL: @func_07
-// CHECK: call float @llvm.experimental.constrained.fsub.f32(float {{.*}}, float {{.*}}, metadata !"round.dynamic", metadata !"fpexcept.strict")
-// CHECK: call float @llvm.experimental.constrained.fmul.f32(float {{.*}}, float {{.*}}, metadata !"round.dynamic", metadata !"fpexcept.strict")
+// STRICT: call float @llvm.experimental.constrained.fsub.f32(float {{.*}}, float {{.*}}, metadata !"round.tonearest", metadata !"fpexcept.strict")
+// STRICT: call float @llvm.experimental.constrained.fmul.f32(float {{.*}}, float {{.*}}, metadata !"round.dynamic", metadata !"fpexcept.strict")
+// STRICT: call float @llvm.experimental.constrained.fadd.f32(float {{.*}}, float {{.*}}, metadata !"round.tonearest", metadata !"fpexcept.strict")
+// DEFAULT: call float @llvm.experimental.constrained.fsub.f32(float {{.*}}, float {{.*}}, metadata !"round.tonearest", metadata !"fpexcept.ignore")
+// DEFAULT: call float @llvm.experimental.constrained.fmul.f32(float {{.*}}, float {{.*}}, metadata !"round.dynamic", metadata !"fpexcept.strict")
+// DEFAULT: call float @llvm.

[PATCH] D126364: Fix interaction of pragma FENV_ACCESS with other pragmas

[Eli Friedman via Phabricator via cfe-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126364

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

[PATCH] D126364: Fix interaction of pragma FENV_ACCESS with other pragmas

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff added inline comments.



Comment at: clang/include/clang/Basic/LangOptions.h:622
 setFPContractMode(LangOptions::FPM_Off);
 setRoundingMode(static_cast(LangOptions::FPR_ToNearest));
 setFPExceptionMode(LangOptions::FPE_Ignore);

efriedma wrote:
> sepavloff wrote:
> > efriedma wrote:
> > > sepavloff wrote:
> > > > efriedma wrote:
> > > > > sepavloff wrote:
> > > > > > efriedma wrote:
> > > > > > > sepavloff wrote:
> > > > > > > > efriedma wrote:
> > > > > > > > > I'm suggesting this should be 
> > > > > > > > > `setRoundingMode(llvm::RoundingMode::Dynamic)`.
> > > > > > > > > 
> > > > > > > > > If FENV_ACCESS is off, getEffectiveRoundingMode() converts 
> > > > > > > > > that to "nearest".  If FENV_ACCESS is turned on, the mode is 
> > > > > > > > > treated as dynamic.  This is exactly what we want.
> > > > > > > > It would change semantics. In particular, it would make 
> > > > > > > > impossible to use FP arithmetic in constexpr functions. An 
> > > > > > > > expression like `1.0 / 3.0` cannot be evaluated with dynamic 
> > > > > > > > rounding mode.
> > > > > > > Can we just change the relevant code in ExprConstant to call 
> > > > > > > getEffectiveRoundingMode()?
> > > > > > What is the advantage of using FE_DYNAMIC in the case when it is 
> > > > > > known that rounding mode is FE_TONEAREST?
> > > > > > 
> > > > > > Probably `FENV_ROUND FE_DYNAMIC` should result in `dynamic` 
> > > > > > rounding even if `FENV_ACCESS ON` is absent. Rounding mode cannot 
> > > > > > be changed in this case but it can be used to inform the compiler 
> > > > > > that such function can be called in environment, where rounding 
> > > > > > mode is non-default. It would prevent some constant evaluations and 
> > > > > > other transformations that assume rounding mode is known. Anyway 
> > > > > > the standard only allow to assume FE_TONEAREST but does not require 
> > > > > > this.
> > > > > > 
> > > > > > In such case `getEffectiveRoundingMode` is not needed.
> > > > > I really want to keep the state we store, and the state updates, as 
> > > > > simple as possible; if that means making getEffectiveRoundingMode() 
> > > > > or whatever more complicated, that's fine.  It's a matter of making 
> > > > > sure we understand all the relevant transitions.
> > > > > 
> > > > > As far as I can tell, according to the standard, the initial state 
> > > > > for FENV_ROUND is supposed to be FE_DYNAMIC, as if "#pragma STDC 
> > > > > FENV_ROUND FE_DYNAMIC" were written at the beginning of every file.  
> > > > > If we think we need a new state, something like FE_DYNAMIC_INITIAL, 
> > > > > to represent the initial state, we can, I guess, but I don't think 
> > > > > the standard text requires it.
> > > > > As far as I can tell, according to the standard, the initial state 
> > > > > for FENV_ROUND is supposed to be FE_DYNAMIC, as if "#pragma STDC 
> > > > > FENV_ROUND FE_DYNAMIC" were written at the beginning of every file. 
> > > > 
> > > > Could you please give any reference to this statement? I thought the 
> > > > initial state should be FE_TONEAREST, as it follows from F.8.3p1.
> > > " If no FENV_ROUND pragma is in effect, or the specified constant 
> > > rounding mode is FE_DYNAMIC, rounding is according to the mode specified 
> > > by the dynamic floating-point environment".  And all the other rules for 
> > > FENV_ROUND only apply to "an FENV_ROUND pragma establishing a mode other 
> > > than FE_DYNAMIC".  So no FENV_ROUND is equivalent to "FENV_ROUND 
> > > FENV_DYNAMIC".
> > > 
> > > The text also says, "If the FE_DYNAMIC mode is specified and FENV_ACCESS 
> > > is 'off', the translator may assume that the default rounding mode is in 
> > > effect".  But that's the same result you'd get if you didn't specify 
> > > FENV_ROUND at all: it's just combining the general rule that you're not 
> > > allowed to mess with the dynamic rounding mode outside the scope of 
> > > FENV_ACCESS, with the rule that the initial rounding mode is "nearest".
> > Thank you for the references. Indeed, default behavior specified by the 
> > standard is to use dynamic rounding. It however do not agree with the 
> > default compiler behavior - to use FE_TONEAREST. That's why we cannot make 
> > `setRoundingMode(llvm::RoundingMode::Dynamic)`. 
> > 
> > With options '-frounding-math' compiler conforms to the latest standard 
> > draft. In this case however FENV_ACCESS is not necessary. Using FE_DYNAMIC 
> > with subsequent deduction of actual rounding mode looks a fragile solution 
> > compared with setting actual mode in CurFPFeatures.
> My goal is to make the state transitions as simple as possible.  It's very 
> easy to mess up state machines.  This means as few variables as possible, in 
> as few states as possible, are involved in parsing the relevant pragmas.
> 
> > It however do not agree with the default compiler behavior - to use 
> > FE_TONEAREST.
> 
> My goal is to make the implementations

[PATCH] D126364: Fix interaction of pragma FENV_ACCESS with other pragmas

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff updated this revision to Diff 437807.
sepavloff added a comment.

Remade the patch according to the review notes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126364

Files:
  clang/include/clang/Basic/FPOptions.def
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Driver/Options.td
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ExprConstant.cpp
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Parse/ParsePragma.cpp
  clang/lib/Sema/SemaAttr.cpp
  clang/test/AST/ast-dump-fpfeatures.cpp
  clang/test/CodeGen/pragma-fenv_access.c

Index: clang/test/CodeGen/pragma-fenv_access.c
===
--- clang/test/CodeGen/pragma-fenv_access.c
+++ clang/test/CodeGen/pragma-fenv_access.c
@@ -1,5 +1,15 @@
-// RUN: %clang_cc1 -fexperimental-strict-floating-point -ffp-exception-behavior=strict -triple %itanium_abi_triple -emit-llvm %s -o - | FileCheck %s
-// RUN: %clang_cc1 -fexperimental-strict-floating-point -ffp-exception-behavior=strict -triple %itanium_abi_triple -emit-llvm %s -o - -fms-extensions -DMS | FileCheck %s
+// RUN: %clang_cc1 -fexperimental-strict-floating-point -ffp-exception-behavior=strict -triple %itanium_abi_triple -emit-llvm %s -o - | FileCheck --check-prefixes=CHECK,STRICT %s
+// RUN: %clang_cc1 -fexperimental-strict-floating-point -ffp-exception-behavior=strict -triple %itanium_abi_triple -emit-llvm %s -o - -fms-extensions -DMS | FileCheck --check-prefixes=CHECK,STRICT %s
+// RUN: %clang_cc1 -fexperimental-strict-floating-point -triple %itanium_abi_triple -emit-llvm %s -o - | FileCheck --check-prefixes=CHECK,DEFAULT %s
+
+
+float func_00(float x, float y) {
+  return x + y;
+}
+// CHECK-LABEL: @func_00
+// STRICT: call float @llvm.experimental.constrained.fadd.f32(float {{.*}}, float {{.*}}, metadata !"round.tonearest", metadata !"fpexcept.strict")
+// DEFAULT: fadd float
+
 
 #ifdef MS
 #pragma fenv_access (on)
@@ -20,7 +30,7 @@
   return x + y;
 }
 // CHECK-LABEL: @func_02
-// CHECK: call float @llvm.experimental.constrained.fadd.f32(float {{.*}}, float {{.*}}, metadata !"round.dynamic", metadata !"fpexcept.ignore")
+// CHECK: call float @llvm.experimental.constrained.fadd.f32(float {{.*}}, float {{.*}}, metadata !"round.tonearest", metadata !"fpexcept.ignore")
 
 
 float func_03(float x, float y) {
@@ -41,7 +51,16 @@
   return x + y;
 }
 // CHECK-LABEL: @func_04
-// CHECK: call float @llvm.experimental.constrained.fadd.f32(float {{.*}}, float {{.*}}, metadata !"round.dynamic", metadata !"fpexcept.ignore")
+// STRICT: call float @llvm.experimental.constrained.fadd.f32(float {{.*}}, float {{.*}}, metadata !"round.tonearest", metadata !"fpexcept.ignore")
+// DEFAULT: fadd float
+
+
+float func_04a(float x, float y) {
+  #pragma float_control(except, on)
+  return x + y;
+}
+// CHECK-LABEL: @func_04a
+// CHECK: call float @llvm.experimental.constrained.fadd.f32(float {{.*}}, float {{.*}}, metadata !"round.tonearest", metadata !"fpexcept.strict")
 
 
 float func_05(float x, float y) {
@@ -57,18 +76,151 @@
   return x + y;
 }
 // CHECK-LABEL: @func_06
-// CHECK: call float @llvm.experimental.constrained.fadd.f32(float {{.*}}, float {{.*}}, metadata !"round.dynamic", metadata !"fpexcept.ignore")
+// STRICT: call float @llvm.experimental.constrained.fadd.f32(float {{.*}}, float {{.*}}, metadata !"round.tonearest", metadata !"fpexcept.ignore")
+// DEFAULT: fadd float
 
 
 float func_07(float x, float y) {
   x -= y;
   if (x) {
 #pragma STDC FENV_ACCESS ON
-y *= 2;
+y *= 2.0F;
   }
-  return y + 4;
+  return y + 4.0F;
 }
 // CHECK-LABEL: @func_07
-// CHECK: call float @llvm.experimental.constrained.fsub.f32(float {{.*}}, float {{.*}}, metadata !"round.dynamic", metadata !"fpexcept.strict")
-// CHECK: call float @llvm.experimental.constrained.fmul.f32(float {{.*}}, float {{.*}}, metadata !"round.dynamic", metadata !"fpexcept.strict")
+// STRICT: call float @llvm.experimental.constrained.fsub.f32(float {{.*}}, float {{.*}}, metadata !"round.tonearest", metadata !"fpexcept.strict")
+// STRICT: call float @llvm.experimental.constrained.fmul.f32(float {{.*}}, float {{.*}}, metadata !"round.dynamic", metadata !"fpexcept.strict")
+// STRICT: call float @llvm.experimental.constrained.fadd.f32(float {{.*}}, float {{.*}}, metadata !"round.tonearest", metadata !"fpexcept.strict")
+// DEFAULT: call float @llvm.experimental.constrained.fsub.f32(float {{.*}}, float {{.*}}, metadata !"round.tonearest", metadata !"fpexcept.ignore")
+// DEFAULT: call float @llvm.experimental.constrained.fmul.f32(float {{.*}}, float {{.*}}, metadata !"round.dynamic", metadata !"fpexcept.strict")
+// DEFAULT: call float @llvm.experimental.constrained.fadd.f32(float {{.*}}, float {{.*}}, metadata !"round.tonearest", metadata !"fpexcept.ignore")
+

[PATCH] D126364: Fix interaction of pragma FENV_ACCESS with other pragmas

[Eli Friedman via Phabricator via cfe-commits]

efriedma added inline comments.



Comment at: clang/include/clang/Basic/LangOptions.h:622
 setFPContractMode(LangOptions::FPM_Off);
 setRoundingMode(static_cast(LangOptions::FPR_ToNearest));
 setFPExceptionMode(LangOptions::FPE_Ignore);

sepavloff wrote:
> efriedma wrote:
> > sepavloff wrote:
> > > efriedma wrote:
> > > > sepavloff wrote:
> > > > > efriedma wrote:
> > > > > > sepavloff wrote:
> > > > > > > efriedma wrote:
> > > > > > > > I'm suggesting this should be 
> > > > > > > > `setRoundingMode(llvm::RoundingMode::Dynamic)`.
> > > > > > > > 
> > > > > > > > If FENV_ACCESS is off, getEffectiveRoundingMode() converts that 
> > > > > > > > to "nearest".  If FENV_ACCESS is turned on, the mode is treated 
> > > > > > > > as dynamic.  This is exactly what we want.
> > > > > > > It would change semantics. In particular, it would make 
> > > > > > > impossible to use FP arithmetic in constexpr functions. An 
> > > > > > > expression like `1.0 / 3.0` cannot be evaluated with dynamic 
> > > > > > > rounding mode.
> > > > > > Can we just change the relevant code in ExprConstant to call 
> > > > > > getEffectiveRoundingMode()?
> > > > > What is the advantage of using FE_DYNAMIC in the case when it is 
> > > > > known that rounding mode is FE_TONEAREST?
> > > > > 
> > > > > Probably `FENV_ROUND FE_DYNAMIC` should result in `dynamic` rounding 
> > > > > even if `FENV_ACCESS ON` is absent. Rounding mode cannot be changed 
> > > > > in this case but it can be used to inform the compiler that such 
> > > > > function can be called in environment, where rounding mode is 
> > > > > non-default. It would prevent some constant evaluations and other 
> > > > > transformations that assume rounding mode is known. Anyway the 
> > > > > standard only allow to assume FE_TONEAREST but does not require this.
> > > > > 
> > > > > In such case `getEffectiveRoundingMode` is not needed.
> > > > I really want to keep the state we store, and the state updates, as 
> > > > simple as possible; if that means making getEffectiveRoundingMode() or 
> > > > whatever more complicated, that's fine.  It's a matter of making sure 
> > > > we understand all the relevant transitions.
> > > > 
> > > > As far as I can tell, according to the standard, the initial state for 
> > > > FENV_ROUND is supposed to be FE_DYNAMIC, as if "#pragma STDC FENV_ROUND 
> > > > FE_DYNAMIC" were written at the beginning of every file.  If we think 
> > > > we need a new state, something like FE_DYNAMIC_INITIAL, to represent 
> > > > the initial state, we can, I guess, but I don't think the standard text 
> > > > requires it.
> > > > As far as I can tell, according to the standard, the initial state for 
> > > > FENV_ROUND is supposed to be FE_DYNAMIC, as if "#pragma STDC FENV_ROUND 
> > > > FE_DYNAMIC" were written at the beginning of every file. 
> > > 
> > > Could you please give any reference to this statement? I thought the 
> > > initial state should be FE_TONEAREST, as it follows from F.8.3p1.
> > " If no FENV_ROUND pragma is in effect, or the specified constant rounding 
> > mode is FE_DYNAMIC, rounding is according to the mode specified by the 
> > dynamic floating-point environment".  And all the other rules for 
> > FENV_ROUND only apply to "an FENV_ROUND pragma establishing a mode other 
> > than FE_DYNAMIC".  So no FENV_ROUND is equivalent to "FENV_ROUND 
> > FENV_DYNAMIC".
> > 
> > The text also says, "If the FE_DYNAMIC mode is specified and FENV_ACCESS is 
> > 'off', the translator may assume that the default rounding mode is in 
> > effect".  But that's the same result you'd get if you didn't specify 
> > FENV_ROUND at all: it's just combining the general rule that you're not 
> > allowed to mess with the dynamic rounding mode outside the scope of 
> > FENV_ACCESS, with the rule that the initial rounding mode is "nearest".
> Thank you for the references. Indeed, default behavior specified by the 
> standard is to use dynamic rounding. It however do not agree with the default 
> compiler behavior - to use FE_TONEAREST. That's why we cannot make 
> `setRoundingMode(llvm::RoundingMode::Dynamic)`. 
> 
> With options '-frounding-math' compiler conforms to the latest standard 
> draft. In this case however FENV_ACCESS is not necessary. Using FE_DYNAMIC 
> with subsequent deduction of actual rounding mode looks a fragile solution 
> compared with setting actual mode in CurFPFeatures.
My goal is to make the state transitions as simple as possible.  It's very easy 
to mess up state machines.  This means as few variables as possible, in as few 
states as possible, are involved in parsing the relevant pragmas.

> It however do not agree with the default compiler behavior - to use 
> FE_TONEAREST.

My goal is to make the implementations of ActOnPragmaFEnvAccess and 
ActOnPragmaFEnvRound as simple as possible.  So the question is, if we store 
the state of FENV_ACCESS as a boolean, and the state of FENV_ROUND as a 
roundi

[PATCH] D126364: Fix interaction of pragma FENV_ACCESS with other pragmas

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff added inline comments.



Comment at: clang/include/clang/Basic/LangOptions.h:622
 setFPContractMode(LangOptions::FPM_Off);
 setRoundingMode(static_cast(LangOptions::FPR_ToNearest));
 setFPExceptionMode(LangOptions::FPE_Ignore);

efriedma wrote:
> sepavloff wrote:
> > efriedma wrote:
> > > sepavloff wrote:
> > > > efriedma wrote:
> > > > > sepavloff wrote:
> > > > > > efriedma wrote:
> > > > > > > I'm suggesting this should be 
> > > > > > > `setRoundingMode(llvm::RoundingMode::Dynamic)`.
> > > > > > > 
> > > > > > > If FENV_ACCESS is off, getEffectiveRoundingMode() converts that 
> > > > > > > to "nearest".  If FENV_ACCESS is turned on, the mode is treated 
> > > > > > > as dynamic.  This is exactly what we want.
> > > > > > It would change semantics. In particular, it would make impossible 
> > > > > > to use FP arithmetic in constexpr functions. An expression like 
> > > > > > `1.0 / 3.0` cannot be evaluated with dynamic rounding mode.
> > > > > Can we just change the relevant code in ExprConstant to call 
> > > > > getEffectiveRoundingMode()?
> > > > What is the advantage of using FE_DYNAMIC in the case when it is known 
> > > > that rounding mode is FE_TONEAREST?
> > > > 
> > > > Probably `FENV_ROUND FE_DYNAMIC` should result in `dynamic` rounding 
> > > > even if `FENV_ACCESS ON` is absent. Rounding mode cannot be changed in 
> > > > this case but it can be used to inform the compiler that such function 
> > > > can be called in environment, where rounding mode is non-default. It 
> > > > would prevent some constant evaluations and other transformations that 
> > > > assume rounding mode is known. Anyway the standard only allow to assume 
> > > > FE_TONEAREST but does not require this.
> > > > 
> > > > In such case `getEffectiveRoundingMode` is not needed.
> > > I really want to keep the state we store, and the state updates, as 
> > > simple as possible; if that means making getEffectiveRoundingMode() or 
> > > whatever more complicated, that's fine.  It's a matter of making sure we 
> > > understand all the relevant transitions.
> > > 
> > > As far as I can tell, according to the standard, the initial state for 
> > > FENV_ROUND is supposed to be FE_DYNAMIC, as if "#pragma STDC FENV_ROUND 
> > > FE_DYNAMIC" were written at the beginning of every file.  If we think we 
> > > need a new state, something like FE_DYNAMIC_INITIAL, to represent the 
> > > initial state, we can, I guess, but I don't think the standard text 
> > > requires it.
> > > As far as I can tell, according to the standard, the initial state for 
> > > FENV_ROUND is supposed to be FE_DYNAMIC, as if "#pragma STDC FENV_ROUND 
> > > FE_DYNAMIC" were written at the beginning of every file. 
> > 
> > Could you please give any reference to this statement? I thought the 
> > initial state should be FE_TONEAREST, as it follows from F.8.3p1.
> " If no FENV_ROUND pragma is in effect, or the specified constant rounding 
> mode is FE_DYNAMIC, rounding is according to the mode specified by the 
> dynamic floating-point environment".  And all the other rules for FENV_ROUND 
> only apply to "an FENV_ROUND pragma establishing a mode other than 
> FE_DYNAMIC".  So no FENV_ROUND is equivalent to "FENV_ROUND FENV_DYNAMIC".
> 
> The text also says, "If the FE_DYNAMIC mode is specified and FENV_ACCESS is 
> 'off', the translator may assume that the default rounding mode is in 
> effect".  But that's the same result you'd get if you didn't specify 
> FENV_ROUND at all: it's just combining the general rule that you're not 
> allowed to mess with the dynamic rounding mode outside the scope of 
> FENV_ACCESS, with the rule that the initial rounding mode is "nearest".
Thank you for the references. Indeed, default behavior specified by the 
standard is to use dynamic rounding. It however do not agree with the default 
compiler behavior - to use FE_TONEAREST. That's why we cannot make 
`setRoundingMode(llvm::RoundingMode::Dynamic)`. 

With options '-frounding-math' compiler conforms to the latest standard draft. 
In this case however FENV_ACCESS is not necessary. Using FE_DYNAMIC with 
subsequent deduction of actual rounding mode looks a fragile solution compared 
with setting actual mode in CurFPFeatures.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126364

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

[PATCH] D126364: Fix interaction of pragma FENV_ACCESS with other pragmas

[Eli Friedman via Phabricator via cfe-commits]

efriedma added inline comments.



Comment at: clang/include/clang/Basic/LangOptions.h:622
 setFPContractMode(LangOptions::FPM_Off);
 setRoundingMode(static_cast(LangOptions::FPR_ToNearest));
 setFPExceptionMode(LangOptions::FPE_Ignore);

sepavloff wrote:
> efriedma wrote:
> > sepavloff wrote:
> > > efriedma wrote:
> > > > sepavloff wrote:
> > > > > efriedma wrote:
> > > > > > I'm suggesting this should be 
> > > > > > `setRoundingMode(llvm::RoundingMode::Dynamic)`.
> > > > > > 
> > > > > > If FENV_ACCESS is off, getEffectiveRoundingMode() converts that to 
> > > > > > "nearest".  If FENV_ACCESS is turned on, the mode is treated as 
> > > > > > dynamic.  This is exactly what we want.
> > > > > It would change semantics. In particular, it would make impossible to 
> > > > > use FP arithmetic in constexpr functions. An expression like `1.0 / 
> > > > > 3.0` cannot be evaluated with dynamic rounding mode.
> > > > Can we just change the relevant code in ExprConstant to call 
> > > > getEffectiveRoundingMode()?
> > > What is the advantage of using FE_DYNAMIC in the case when it is known 
> > > that rounding mode is FE_TONEAREST?
> > > 
> > > Probably `FENV_ROUND FE_DYNAMIC` should result in `dynamic` rounding even 
> > > if `FENV_ACCESS ON` is absent. Rounding mode cannot be changed in this 
> > > case but it can be used to inform the compiler that such function can be 
> > > called in environment, where rounding mode is non-default. It would 
> > > prevent some constant evaluations and other transformations that assume 
> > > rounding mode is known. Anyway the standard only allow to assume 
> > > FE_TONEAREST but does not require this.
> > > 
> > > In such case `getEffectiveRoundingMode` is not needed.
> > I really want to keep the state we store, and the state updates, as simple 
> > as possible; if that means making getEffectiveRoundingMode() or whatever 
> > more complicated, that's fine.  It's a matter of making sure we understand 
> > all the relevant transitions.
> > 
> > As far as I can tell, according to the standard, the initial state for 
> > FENV_ROUND is supposed to be FE_DYNAMIC, as if "#pragma STDC FENV_ROUND 
> > FE_DYNAMIC" were written at the beginning of every file.  If we think we 
> > need a new state, something like FE_DYNAMIC_INITIAL, to represent the 
> > initial state, we can, I guess, but I don't think the standard text 
> > requires it.
> > As far as I can tell, according to the standard, the initial state for 
> > FENV_ROUND is supposed to be FE_DYNAMIC, as if "#pragma STDC FENV_ROUND 
> > FE_DYNAMIC" were written at the beginning of every file. 
> 
> Could you please give any reference to this statement? I thought the initial 
> state should be FE_TONEAREST, as it follows from F.8.3p1.
" If no FENV_ROUND pragma is in effect, or the specified constant rounding mode 
is FE_DYNAMIC, rounding is according to the mode specified by the dynamic 
floating-point environment".  And all the other rules for FENV_ROUND only apply 
to "an FENV_ROUND pragma establishing a mode other than FE_DYNAMIC".  So no 
FENV_ROUND is equivalent to "FENV_ROUND FENV_DYNAMIC".

The text also says, "If the FE_DYNAMIC mode is specified and FENV_ACCESS is 
'off', the translator may assume that the default rounding mode is in effect".  
But that's the same result you'd get if you didn't specify FENV_ROUND at all: 
it's just combining the general rule that you're not allowed to mess with the 
dynamic rounding mode outside the scope of FENV_ACCESS, with the rule that the 
initial rounding mode is "nearest".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126364

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

[PATCH] D126364: Fix interaction of pragma FENV_ACCESS with other pragmas

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff added inline comments.



Comment at: clang/include/clang/Basic/LangOptions.h:622
 setFPContractMode(LangOptions::FPM_Off);
 setRoundingMode(static_cast(LangOptions::FPR_ToNearest));
 setFPExceptionMode(LangOptions::FPE_Ignore);

efriedma wrote:
> sepavloff wrote:
> > efriedma wrote:
> > > sepavloff wrote:
> > > > efriedma wrote:
> > > > > I'm suggesting this should be 
> > > > > `setRoundingMode(llvm::RoundingMode::Dynamic)`.
> > > > > 
> > > > > If FENV_ACCESS is off, getEffectiveRoundingMode() converts that to 
> > > > > "nearest".  If FENV_ACCESS is turned on, the mode is treated as 
> > > > > dynamic.  This is exactly what we want.
> > > > It would change semantics. In particular, it would make impossible to 
> > > > use FP arithmetic in constexpr functions. An expression like `1.0 / 
> > > > 3.0` cannot be evaluated with dynamic rounding mode.
> > > Can we just change the relevant code in ExprConstant to call 
> > > getEffectiveRoundingMode()?
> > What is the advantage of using FE_DYNAMIC in the case when it is known that 
> > rounding mode is FE_TONEAREST?
> > 
> > Probably `FENV_ROUND FE_DYNAMIC` should result in `dynamic` rounding even 
> > if `FENV_ACCESS ON` is absent. Rounding mode cannot be changed in this case 
> > but it can be used to inform the compiler that such function can be called 
> > in environment, where rounding mode is non-default. It would prevent some 
> > constant evaluations and other transformations that assume rounding mode is 
> > known. Anyway the standard only allow to assume FE_TONEAREST but does not 
> > require this.
> > 
> > In such case `getEffectiveRoundingMode` is not needed.
> I really want to keep the state we store, and the state updates, as simple as 
> possible; if that means making getEffectiveRoundingMode() or whatever more 
> complicated, that's fine.  It's a matter of making sure we understand all the 
> relevant transitions.
> 
> As far as I can tell, according to the standard, the initial state for 
> FENV_ROUND is supposed to be FE_DYNAMIC, as if "#pragma STDC FENV_ROUND 
> FE_DYNAMIC" were written at the beginning of every file.  If we think we need 
> a new state, something like FE_DYNAMIC_INITIAL, to represent the initial 
> state, we can, I guess, but I don't think the standard text requires it.
> As far as I can tell, according to the standard, the initial state for 
> FENV_ROUND is supposed to be FE_DYNAMIC, as if "#pragma STDC FENV_ROUND 
> FE_DYNAMIC" were written at the beginning of every file. 

Could you please give any reference to this statement? I thought the initial 
state should be FE_TONEAREST, as it follows from F.8.3p1.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126364

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

[PATCH] D126364: Fix interaction of pragma FENV_ACCESS with other pragmas

[Eli Friedman via Phabricator via cfe-commits]

efriedma added inline comments.



Comment at: clang/include/clang/Basic/LangOptions.h:622
 setFPContractMode(LangOptions::FPM_Off);
 setRoundingMode(static_cast(LangOptions::FPR_ToNearest));
 setFPExceptionMode(LangOptions::FPE_Ignore);

sepavloff wrote:
> efriedma wrote:
> > sepavloff wrote:
> > > efriedma wrote:
> > > > I'm suggesting this should be 
> > > > `setRoundingMode(llvm::RoundingMode::Dynamic)`.
> > > > 
> > > > If FENV_ACCESS is off, getEffectiveRoundingMode() converts that to 
> > > > "nearest".  If FENV_ACCESS is turned on, the mode is treated as 
> > > > dynamic.  This is exactly what we want.
> > > It would change semantics. In particular, it would make impossible to use 
> > > FP arithmetic in constexpr functions. An expression like `1.0 / 3.0` 
> > > cannot be evaluated with dynamic rounding mode.
> > Can we just change the relevant code in ExprConstant to call 
> > getEffectiveRoundingMode()?
> What is the advantage of using FE_DYNAMIC in the case when it is known that 
> rounding mode is FE_TONEAREST?
> 
> Probably `FENV_ROUND FE_DYNAMIC` should result in `dynamic` rounding even if 
> `FENV_ACCESS ON` is absent. Rounding mode cannot be changed in this case but 
> it can be used to inform the compiler that such function can be called in 
> environment, where rounding mode is non-default. It would prevent some 
> constant evaluations and other transformations that assume rounding mode is 
> known. Anyway the standard only allow to assume FE_TONEAREST but does not 
> require this.
> 
> In such case `getEffectiveRoundingMode` is not needed.
I really want to keep the state we store, and the state updates, as simple as 
possible; if that means making getEffectiveRoundingMode() or whatever more 
complicated, that's fine.  It's a matter of making sure we understand all the 
relevant transitions.

As far as I can tell, according to the standard, the initial state for 
FENV_ROUND is supposed to be FE_DYNAMIC, as if "#pragma STDC FENV_ROUND 
FE_DYNAMIC" were written at the beginning of every file.  If we think we need a 
new state, something like FE_DYNAMIC_INITIAL, to represent the initial state, 
we can, I guess, but I don't think the standard text requires it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126364

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

[PATCH] D126364: Fix interaction of pragma FENV_ACCESS with other pragmas

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff added inline comments.



Comment at: clang/include/clang/Basic/LangOptions.h:622
 setFPContractMode(LangOptions::FPM_Off);
 setRoundingMode(static_cast(LangOptions::FPR_ToNearest));
 setFPExceptionMode(LangOptions::FPE_Ignore);

efriedma wrote:
> sepavloff wrote:
> > efriedma wrote:
> > > I'm suggesting this should be 
> > > `setRoundingMode(llvm::RoundingMode::Dynamic)`.
> > > 
> > > If FENV_ACCESS is off, getEffectiveRoundingMode() converts that to 
> > > "nearest".  If FENV_ACCESS is turned on, the mode is treated as dynamic.  
> > > This is exactly what we want.
> > It would change semantics. In particular, it would make impossible to use 
> > FP arithmetic in constexpr functions. An expression like `1.0 / 3.0` cannot 
> > be evaluated with dynamic rounding mode.
> Can we just change the relevant code in ExprConstant to call 
> getEffectiveRoundingMode()?
What is the advantage of using FE_DYNAMIC in the case when it is known that 
rounding mode is FE_TONEAREST?

Probably `FENV_ROUND FE_DYNAMIC` should result in `dynamic` rounding even if 
`FENV_ACCESS ON` is absent. Rounding mode cannot be changed in this case but it 
can be used to inform the compiler that such function can be called in 
environment, where rounding mode is non-default. It would prevent some constant 
evaluations and other transformations that assume rounding mode is known. 
Anyway the standard only allow to assume FE_TONEAREST but does not require this.

In such case `getEffectiveRoundingMode` is not needed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126364

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

[PATCH] D126364: Fix interaction of pragma FENV_ACCESS with other pragmas

[Eli Friedman via Phabricator via cfe-commits]

efriedma added inline comments.



Comment at: clang/include/clang/Basic/LangOptions.h:622
 setFPContractMode(LangOptions::FPM_Off);
 setRoundingMode(static_cast(LangOptions::FPR_ToNearest));
 setFPExceptionMode(LangOptions::FPE_Ignore);

sepavloff wrote:
> efriedma wrote:
> > I'm suggesting this should be 
> > `setRoundingMode(llvm::RoundingMode::Dynamic)`.
> > 
> > If FENV_ACCESS is off, getEffectiveRoundingMode() converts that to 
> > "nearest".  If FENV_ACCESS is turned on, the mode is treated as dynamic.  
> > This is exactly what we want.
> It would change semantics. In particular, it would make impossible to use FP 
> arithmetic in constexpr functions. An expression like `1.0 / 3.0` cannot be 
> evaluated with dynamic rounding mode.
Can we just change the relevant code in ExprConstant to call 
getEffectiveRoundingMode()?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126364

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

[PATCH] D126364: Fix interaction of pragma FENV_ACCESS with other pragmas

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff added inline comments.



Comment at: clang/include/clang/Basic/LangOptions.h:622
 setFPContractMode(LangOptions::FPM_Off);
 setRoundingMode(static_cast(LangOptions::FPR_ToNearest));
 setFPExceptionMode(LangOptions::FPE_Ignore);

efriedma wrote:
> I'm suggesting this should be `setRoundingMode(llvm::RoundingMode::Dynamic)`.
> 
> If FENV_ACCESS is off, getEffectiveRoundingMode() converts that to "nearest". 
>  If FENV_ACCESS is turned on, the mode is treated as dynamic.  This is 
> exactly what we want.
It would change semantics. In particular, it would make impossible to use FP 
arithmetic in constexpr functions. An expression like `1.0 / 3.0` cannot be 
evaluated with dynamic rounding mode.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126364

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

[PATCH] D126364: Fix interaction of pragma FENV_ACCESS with other pragmas

[Eli Friedman via Phabricator via cfe-commits]

efriedma added inline comments.



Comment at: clang/include/clang/Basic/LangOptions.h:622
 setFPContractMode(LangOptions::FPM_Off);
 setRoundingMode(static_cast(LangOptions::FPR_ToNearest));
 setFPExceptionMode(LangOptions::FPE_Ignore);

I'm suggesting this should be `setRoundingMode(llvm::RoundingMode::Dynamic)`.

If FENV_ACCESS is off, getEffectiveRoundingMode() converts that to "nearest".  
If FENV_ACCESS is turned on, the mode is treated as dynamic.  This is exactly 
what we want.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126364

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

[PATCH] D126364: Fix interaction of pragma FENV_ACCESS with other pragmas

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff added a comment.

In D126364#3560997 , @efriedma wrote:

> In D126364#3560984 , @sepavloff 
> wrote:
>
>> In D126364#3560877 , @efriedma 
>> wrote:
>>
>>> Shouldn't the rounding mode be FE_DYNAMIC by default?
>>
>> According to the standard it must be FE_TONEAREST:
>>
>>   F.8.3p1:
>>   At program startup the dynamic floating-point environment is initialized 
>> as prescribed by IEC 60559:
>>   ...
>>   - The dynamic rounding direction mode is rounding to nearest.
>>   ...
>
> When I say the default should be FE_DYNAMIC, I'm talking about the default 
> FENV_ROUND state.  That's talking about the initial state of the 
> floating-point registers at runtime (on entry to main()).

The cited excerpt from the standard is just about floating point environment at 
program startup.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126364

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

[PATCH] D126364: Fix interaction of pragma FENV_ACCESS with other pragmas

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

In D126364#3560984 , @sepavloff wrote:

> In D126364#3560877 , @efriedma 
> wrote:
>
>> Shouldn't the rounding mode be FE_DYNAMIC by default?
>
> According to the standard it must be FE_TONEAREST:
>
>   F.8.3p1:
>   At program startup the dynamic floating-point environment is initialized as 
> prescribed by IEC 60559:
>   ...
>   - The dynamic rounding direction mode is rounding to nearest.
>   ...

When I say the default should be FE_DYNAMIC, I'm talking about the default 
FENV_ROUND state.  That's talking about the initial state of the floating-point 
registers at runtime (on entry to main()).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126364

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

[PATCH] D126364: Fix interaction of pragma FENV_ACCESS with other pragmas

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff added a comment.

In D126364#3560877 , @efriedma wrote:

> Shouldn't the rounding mode be FE_DYNAMIC by default?

According to the standard it must be FE_TONEAREST:

  F.8.3p1:
  At program startup the dynamic floating-point environment is initialized as 
prescribed by IEC 60559:
  ...
  - The dynamic rounding direction mode is rounding to nearest.
  ...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126364

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

[PATCH] D126364: Fix interaction of pragma FENV_ACCESS with other pragmas

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

Shouldn't the rounding mode be FE_DYNAMIC by default?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126364

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

[PATCH] D126364: Fix interaction of pragma FENV_ACCESS with other pragmas

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff added a comment.

In D126364#3556943 , @efriedma wrote:

> Given we have getEffectiveRoundingMode(), I think the calls to 
> setRoundingModeOverride shouldn't be necessary?  And if we drop those calls, 
> we can also drop the IsRoundingModeSet variable.

In the block:

  {
  #pragma STDC FENV_ACCESS ON
  fesetround(x);
  ...
  }

floating point operations must have rounding mode `dynamic`. If handler of 
FENV_ACCESS does not set it, the rounding mode remains FE_TONEAREST, which is 
incorrect.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126364

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

[PATCH] D126364: Fix interaction of pragma FENV_ACCESS with other pragmas

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

Given we have getEffectiveRoundingMode(), I think the calls to 
setRoundingModeOverride shouldn't be necessary?  And if we drop those calls, we 
can also drop the IsRoundingModeSet variable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126364

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

[PATCH] D126364: Fix interaction of pragma FENV_ACCESS with other pragmas

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff added a comment.

In D126364#3548815 , @efriedma wrote:

> The way I see it, there are two possibilities here:
>
> 1. In Sema, we have two rounding modes that correspond to FE_DYNAMIC: 
> llvm::RoundingMode::Dynamic, and llvm::RoundingMode::NearestTiesToEven, plus 
> some boolean to indicate whether the user actually explicitly specified 
> FE_TONEAREST.  (You're currently missing the boolean, which means that 
> currently "#pragma STDC FENV_ACCESS OFF" followed by "#pragma STDC 
> FENV_ACCESS ON" actually mutates the rounding mode.) We juggle the rounding 
> modes and the bit based on whether FENV_ACCESS is currently enabled.
> 2. We just have one rounding mode in Sema that corresponds to FE_DYNAMIC.  
> Then in CodeGen, we set the actual rounding mode based on whether FENV_ACCESS 
> is currently enabled.
>
> (2) seems a lot simpler.

It makes sense. Updated the patch accordingly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126364

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

[PATCH] D126364: Fix interaction of pragma FENV_ACCESS with other pragmas

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff updated this revision to Diff 433963.
sepavloff added a comment.

Update the patch

- Add a flag to reflect rounding mode change by FENV_ROUND,
- Keep dynamic mode in AST and change it later in CodeGen,
- Add new tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126364

Files:
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Sema/Sema.h
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/Parse/ParsePragma.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaAttr.cpp
  clang/test/AST/ast-dump-fpfeatures.cpp
  clang/test/CodeGen/pragma-fenv_access.c

Index: clang/test/CodeGen/pragma-fenv_access.c
===
--- clang/test/CodeGen/pragma-fenv_access.c
+++ clang/test/CodeGen/pragma-fenv_access.c
@@ -20,7 +20,7 @@
   return x + y;
 }
 // CHECK-LABEL: @func_02
-// CHECK: call float @llvm.experimental.constrained.fadd.f32(float {{.*}}, float {{.*}}, metadata !"round.dynamic", metadata !"fpexcept.ignore")
+// CHECK: call float @llvm.experimental.constrained.fadd.f32(float {{.*}}, float {{.*}}, metadata !"round.tonearest", metadata !"fpexcept.ignore")
 
 
 float func_03(float x, float y) {
@@ -41,7 +41,15 @@
   return x + y;
 }
 // CHECK-LABEL: @func_04
-// CHECK: call float @llvm.experimental.constrained.fadd.f32(float {{.*}}, float {{.*}}, metadata !"round.dynamic", metadata !"fpexcept.ignore")
+// CHECK: fadd float
+
+
+float func_04a(float x, float y) {
+  #pragma float_control(except, on)
+  return x + y;
+}
+// CHECK-LABEL: @func_04a
+// CHECK: call float @llvm.experimental.constrained.fadd.f32(float {{.*}}, float {{.*}}, metadata !"round.tonearest", metadata !"fpexcept.strict")
 
 
 float func_05(float x, float y) {
@@ -57,18 +65,117 @@
   return x + y;
 }
 // CHECK-LABEL: @func_06
-// CHECK: call float @llvm.experimental.constrained.fadd.f32(float {{.*}}, float {{.*}}, metadata !"round.dynamic", metadata !"fpexcept.ignore")
+// CHECK: fadd float
 
 
 float func_07(float x, float y) {
   x -= y;
   if (x) {
 #pragma STDC FENV_ACCESS ON
-y *= 2;
+y *= 2.0F;
   }
-  return y + 4;
+  return y + 4.0F;
 }
 // CHECK-LABEL: @func_07
-// CHECK: call float @llvm.experimental.constrained.fsub.f32(float {{.*}}, float {{.*}}, metadata !"round.dynamic", metadata !"fpexcept.strict")
+// CHECK: call float @llvm.experimental.constrained.fsub.f32(float {{.*}}, float {{.*}}, metadata !"round.tonearest", metadata !"fpexcept.ignore")
 // CHECK: call float @llvm.experimental.constrained.fmul.f32(float {{.*}}, float {{.*}}, metadata !"round.dynamic", metadata !"fpexcept.strict")
+// CHECK: call float @llvm.experimental.constrained.fadd.f32(float {{.*}}, float {{.*}}, metadata !"round.tonearest", metadata !"fpexcept.ignore")
+
+
+float func_08(float x, float y) {
+  #pragma STDC FENV_ROUND FE_UPWARD
+  #pragma STDC FENV_ACCESS ON
+  return x + y;
+}
+// CHECK-LABEL: @func_08
+// CHECK:  call float @llvm.experimental.constrained.fadd.f32({{.*}}, metadata !"round.upward", metadata !"fpexcept.strict")
+
+
+float func_08a(float x, float y) {
+  #pragma STDC FENV_ROUND FE_TONEAREST
+  #pragma STDC FENV_ACCESS ON
+  return x + y;
+}
+// CHECK-LABEL: @func_08a
+// CHECK: call float @llvm.experimental.constrained.fadd.f32(float {{.*}}, float {{.*}}, metadata !"round.tonearest", metadata !"fpexcept.strict")
+
+
+float func_09(float x, float y) {
+  #pragma clang fp exceptions(maytrap)
+  #pragma STDC FENV_ACCESS ON
+  return x + y;
+}
+// CHECK-LABEL: @func_09
+// CHECK:  call float @llvm.experimental.constrained.fadd.f32({{.*}}, metadata !"round.dynamic", metadata !"fpexcept.maytrap")
+
+
+float func_10(float x, float y) {
+  #pragma clang fp exceptions(maytrap)
+  #pragma STDC FENV_ROUND FE_UPWARD
+  #pragma STDC FENV_ACCESS ON
+  return x + y;
+}
+// CHECK-LABEL: @func_10
+// CHECK:  call float @llvm.experimental.constrained.fadd.f32({{.*}}, metadata !"round.upward", metadata !"fpexcept.maytrap")
+
+
+float func_11(float x, float y, float z) {
+  #pragma STDC FENV_ACCESS ON
+  float res = x * y;
+  {
+#pragma STDC FENV_ACCESS OFF
+return res + z;
+  }
+}
+// CHECK-LABEL: @func_11
+// CHECK:  call float @llvm.experimental.constrained.fmul.f32({{.*}}, metadata !"round.dynamic", metadata !"fpexcept.strict")
+// CHECK:  call float @llvm.experimental.constrained.fadd.f32({{.*}}, metadata !"round.tonearest", metadata !"fpexcept.ignore")
+
+
+float func_12(float x, float y, float z) {
+  #pragma STDC FENV_ROUND FE_TOWARDZERO
+  #pragma STDC FENV_ACCESS ON
+  float res = x * y;
+  {
+#pragma STDC FENV_ACCESS OFF
+return res + z;
+  }
+}
+// CHECK-LABEL: @func_12
+// CHECK:  call float @llvm.experimental.constrained.fmul.f32({{.*}}, metadata !"round.towardzero", metadata !"fpexcept.strict")
+// CHECK:  call float @llvm.experimental.constrained.fadd.f32({{.*}}, metadata !"round.towardzero", metadata !"fpexcept.ignore")
+
+
+float func_13(float x, floa

[PATCH] D126364: Fix interaction of pragma FENV_ACCESS with other pragmas

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

> Constrained intrinsics don't change the rounding mode.

The standard text for FENV_ROUND requires that when we see a floating point 
operation, we have to perform it using the specified rounding mode.  
"floating-point operators [...] shall be evaluated according to the specified 
constant rounding mode (as though no constant mode was specified and the 
corresponding dynamic rounding mode had been established by a call to 
fesetround)".  Like you've noted, we don't have any way to represent that 
directly in LLVM IR.  So the frontend probably needs to emit code to modify the 
rounding mode.

> Because of the requirement that the rounding mode be changed already, I don't 
> see how instructions with static rounding modes are generally interesting. 
> Perhaps a target will learn to recognize that a sequence of instructions can 
> use the static rounding in the instructions and elide a change of rounding 
> mode around the sequence? I don't know how common that would be in practice.

If you're using FENV_ROUND, presumably sequences involving rounding mode 
changes become more common.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126364

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

[PATCH] D126364: Fix interaction of pragma FENV_ACCESS with other pragmas

[Kevin P. Neal via Phabricator via cfe-commits]

kpn added a comment.

>> On targets that support static rounding mode (like RISCV) dynamic and 
>> constant rounding modes may be different and the behavior changes if default 
>> mode is replaced by dynamic.
>
> Whether a target supports static rounding modes on floating-point 
> instructions is completely irrelevant.  The user-visible behavior must be the 
> same either way.  If a target doesn't have specialized instructions, the code 
> generator can save/restore the rounding mode.  This should be transparent to 
> the user; the user can't read the rounding mode in between the save and 
> restore.  (We already do this sort of rounding mode manipulation on x86, to 
> implement float-to-int conversion on targets without SSE.)

The rounding mode argument to the constrained intrinsic is a hint; it tells the 
intrinsic what rounding mode has been set via the normal rounding mode changing 
mechanism. Constrained intrinsics don't change the rounding mode. I guess we 
could weaken that by adding "... except if absolutely required" to account for 
the float-to-int sans SSE case, but the principle applies.

Because of the requirement that the rounding mode be changed already, I don't 
see how instructions with static rounding modes are generally interesting. 
Perhaps a target will learn to recognize that a sequence of instructions can 
use the static rounding in the instructions and elide a change of rounding mode 
around the sequence? I don't know how common that would be in practice.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126364

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

[PATCH] D126364: Fix interaction of pragma FENV_ACCESS with other pragmas

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

The way I see it, there are two possibilities here:

1. In Sema, we have two rounding modes that correspond to FE_DYNAMIC: 
llvm::RoundingMode::Dynamic, and llvm::RoundingMode::NearestTiesToEven, plus 
some boolean to indicate whether the user actually explicitly specified 
FE_TONEAREST.  (You're currently missing the boolean, which means that 
currently "#pragma STDC FENV_ACCESS OFF" followed by "#pragma STDC FENV_ACCESS 
ON" actually mutates the rounding mode.) We juggle the rounding modes and the 
bit based on whether FENV_ACCESS is currently enabled.
2. We just have one rounding mode in Sema that corresponds to FE_DYNAMIC.  Then 
in CodeGen, we set the actual rounding mode based on whether FENV_ACCESS is 
currently enabled.

(2) seems a lot simpler.

> On targets that support static rounding mode (like RISCV) dynamic and 
> constant rounding modes may be different and the behavior changes if default 
> mode is replaced by dynamic.

Whether a target supports static rounding modes on floating-point instructions 
is completely irrelevant.  The user-visible behavior must be the same either 
way.  If a target doesn't have specialized instructions, the code generator can 
save/restore the rounding mode.  This should be transparent to the user; the 
user can't read the rounding mode in between the save and restore.  (We already 
do this sort of rounding mode manipulation on x86, to implement float-to-int 
conversion on targets without SSE.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126364

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

[PATCH] D126364: Fix interaction of pragma FENV_ACCESS with other pragmas

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff added a comment.

In D126364#3546173 , @efriedma wrote:

>> setRoundingMode definitely should not call getAllowFEnvAccess() and it does 
>> not
>
> Yes, it does?
>
>   // C2x: 7.6.2p3  If the FE_DYNAMIC mode is specified and FENV_ACCESS is 
> "off",
>   // the translator may assume that the default rounding mode is in effect.
>   if (FPR == llvm::RoundingMode::Dynamic &&
>   !CurFPFeatures.getAllowFEnvAccess() &&
>   CurFPFeatures.getFPExceptionMode() == LangOptions::FPE_Ignore)
> FPR = llvm::RoundingMode::NearestTiesToEven;

Sorry for my confusion, I meant setAllowFEnvAccess().

>> As for ActOnPragmaFEnvAccess, it make sense to set the most general mode.
>
> Shouldn't the default mode just be "dynamic"?  The fact that we emit 
> non-strict floating-point ops if FENV_ACCESS is disabled is an implementation 
> detail.  I don't see the point of switching the rounding mode between 
> "Dynamic" and "NearestTiesToEven" depending on whether FENV_ACCESS is 
> enabled.  Making the required state transitions complicated like this just 
> makes the code harder to understand.

The standard says that

  the translator may assume that the default rounding mode is in effect

so it does not require to make such assumption, and we can use the most 
profitable way.

The advantages of using default rounding mode are:

1. In this case the mode has concrete value. Constant evaluator can evaluate 
expressions like `1.0/3.0`, which is not allowed if rounding is dynamic, and 
the expression is not constexpr anymore.

2. On targets that support static rounding mode (like RISCV) dynamic and 
constant rounding modes may be different and the behavior changes if default 
mode is replaced by dynamic.

In these cases choosing dynamic mode is not an implementation details, it has 
user-visible effect and the behavior should be chosen most close to the 
standard.

It seems that `#pragma STDC FENV_ROUND FE_DYNAMIC` without `#pragma STDC 
FENV_ACCESS ON` is more like an error, because there is no legitimate way to 
set rounding mode in this case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126364

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

[PATCH] D126364: Fix interaction of pragma FENV_ACCESS with other pragmas

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

> setRoundingMode definitely should not call getAllowFEnvAccess() and it does 
> not

Yes, it does?

  // C2x: 7.6.2p3  If the FE_DYNAMIC mode is specified and FENV_ACCESS is "off",
  // the translator may assume that the default rounding mode is in effect.
  if (FPR == llvm::RoundingMode::Dynamic &&
  !CurFPFeatures.getAllowFEnvAccess() &&
  CurFPFeatures.getFPExceptionMode() == LangOptions::FPE_Ignore)
FPR = llvm::RoundingMode::NearestTiesToEven;



> As for ActOnPragmaFEnvAccess, it make sense to set the most general mode.

Shouldn't the default mode just be "dynamic"?  The fact that we emit non-strict 
floating-point ops if FENV_ACCESS is disabled is an implementation detail.  I 
don't see the point of switching the rounding mode between "Dynamic" and 
"NearestTiesToEven" depending on whether FENV_ACCESS is enabled.  Making the 
required state transitions complicated like this just makes the code harder to 
understand.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126364

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

[PATCH] D126364: Fix interaction of pragma FENV_ACCESS with other pragmas

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff added a comment.

In D126364#3542917 , @efriedma wrote:

> I mean that ActOnPragmaFEnvAccess shouldn't call 
> hasRoundingModeOverride()/setRoundingModeOverride(), and setRoundingMode 
> shouldn't call getAllowFEnvAccess().

setRoundingMode definitely should not call getAllowFEnvAccess() and it does 
not. As for ActOnPragmaFEnvAccess, it make sense to set the most general mode. 
Code like:

  {
  #pragma STDC FENV_ACCESS ON
  fesetround(x);
  ...
  }

must work as is, and it requires that operations are created with dynamic 
rounding mode.

If a user wants to have more specific mode, they can amend FENV_ACCESS with 
other pragmas, like FENV_ROUND or `clang fp exceptions`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126364

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

[PATCH] D126364: Fix interaction of pragma FENV_ACCESS with other pragmas

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

I mean that ActOnPragmaFEnvAccess shouldn't call 
hasRoundingModeOverride()/setRoundingModeOverride(), and setRoundingMode 
shouldn't call getAllowFEnvAccess().


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126364

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

[PATCH] D126364: Fix interaction of pragma FENV_ACCESS with other pragmas

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff added a comment.

In D126364#3540673 , @efriedma wrote:

> For FENV_ROUND, I think we should try to stick to the standard as closely as 
> possible in Sema; since the standard models FENV_ROUND as a separate state, 
> Sema should also model it as a separate state.

I try to figure out the difference between rounding mode as specified by 
FENV_ROUND and that represented by `Sema::CufFPFeatures`. The standard uses two 
notions for rounding mode:

- dynamic rounding mode, which is actually bits in a control register, and
- constant rounding mode, which is used for floating-point operators, implicit 
conversions and so on.

Dynamic rounding mode is set/queried by fegetround/fesetround/FLT_ROUNDS. It is 
unknown at compile time except that in default FP mode it is expected to be 
FE_TONEAREST.

What compiler can know is constant rounding mode, it is just the mode set by 
pragma FENV_ROUND.

It looks like `Sema::CufFPFeatures` is exactly what FENV_ROUND controls.
Do I miss something?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126364

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

[PATCH] D126364: Fix interaction of pragma FENV_ACCESS with other pragmas

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

For FENV_ROUND, I think we should try to stick to the standard as closely as 
possible in Sema; since the standard models FENV_ROUND as a separate state, 
Sema should also model it as a separate state.  If CodeGen decides it needs to 
modify the FP environment to actually implement the standard's rules, it can 
change the way it generates code to match.  (The example in 7.6.2p5 is just 
pseudo-code; any internal state changes should be transparent to the user.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126364

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

[PATCH] D126364: Fix interaction of pragma FENV_ACCESS with other pragmas

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff added a comment.

In D126364#3537964 , @efriedma wrote:

> Could you lay out the expected interaction between "STDC FENV_ACCESS", "clang 
> fp exceptions", "float_control", and "fenv_access"?  If there's some way to 
> map everything to "#pragma clang fp", please lay that out; if that isn't 
> possible, please explain why.

`pragma fenv_access` is same as `pragma STDC FENV_ACCESS`.
`pragma float_control(except, ...)` is same as `pragma clang fp exception` 
amended with possibility to maintain stack of the directives using `push` token.
So indeed, all cases can be mapped to the interaction of  `pragma STDC 
FENV_ACCESS` with `pragma clang fp exception`.
Possible combinations are:

1. No access to FP environment:

#pragma STDC FENV_ACCESS OFF
#pragma clang fp exception(ignore)  // maytrap or strict is not allowed

2. Strict exception mode:

#pragma STDC FENV_ACCESS ON
#pragma clang fp exception(strict)

3. MayTrap exception mode:

#pragma STDC FENV_ACCESS ON
#pragma clang fp exception(maytrap)

4. Ignore exception mode

#pragma STDC FENV_ACCESS ON
#pragma clang fp exception(ignore)

In this mode the code may change FP control modes, like rounding mode, but FP 
exceptions are ignored.
All of them must be supported.

> As far as I can tell, "STDC FENV_ACCESS" and "STDC FENV_ROUND" don't directly 
> interact.  FENV_ROUND just overrides the rounding mode for specific 
> floating-point operations; it doesn't impact whether environment access is 
> allowed.  So the call to setRoundingModeOverride seems dubious.

According to the latest standard draft 
http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2731.pdf:

  7.6.2p2
  The FENV_ROUND pragma provides a means to specify a constant rounding 
direction for floating-point
  operations for standard floating types ...
  
  7.6.2p4
  ... Within the scope of an FENV_ROUND pragma establishing a mode other than 
FE_DYNAMIC, floating-point operators, implicit conversions ..., and invocations 
of functions indicated in the table below, ...  shall be evaluated according to 
the specified constant rounding mode (as though no constant mode was specified 
and the corresponding dynamic rounding mode had been established by a call to 
fesetround).

So indeed the pragmas are independent, with the exception:

  7.6.2p3
  ... If the FE_DYNAMIC mode is specified and FENV_ACCESS is "off", the 
translator may assume that the default rounding mode is in effect.

If target allows specifying rounding mode in instruction (like RISCV), 
FENV_ROUND may be implemented without changing FP environment. However for 
targets, where change of rounding mode requires modification of a control 
register, there is no other way. The standard explicitly allows it in:

  7.6.2p5
  NOTE Constant rounding modes (other than FE_DYNAMIC) could be implemented 
using dynamic rounding modes as
  illustrated in the following example:
  {
  #pragma STDC FENV_ROUND direction
  // compiler inserts:
  // #pragma STDC FENV_ACCESS ON
  // int __savedrnd;
  // __savedrnd = __swapround(direction);
  ... operations affected by constant rounding mode ...
  // compiler inserts:
  // __savedrnd = __swapround(__savedrnd);
  ... operations not affected by constant rounding mode ...
  // compiler inserts:
  // __savedrnd = __swapround(__savedrnd);
  ... operations affected by constant rounding mode ...
  // compiler inserts:
  // __swapround(__savedrnd);
  }
  
  where __swapround is defined by:
  
  static inline int __swapround(const int new) {
  const int old = fegetround();
  fesetround(new);
  return old;
  }

As it follows from the standard, FENV_ROUND in general case changes FP 
environment (sets rounding mode). Also it affects any FP operation in the scope 
of the pragma, which is modelled as current rounding mode in clang. So the call 
to setRoundingModeOverride seems necessary.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126364

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

[PATCH] D126364: Fix interaction of pragma FENV_ACCESS with other pragmas

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

Could you lay out the expected interaction between "STDC FENV_ACCESS", "clang 
fp exceptions", "float_control", and "fenv_access"?  If there's some way to map 
everything to "#pragma clang fp", please lay that out; if that isn't possible, 
please explain why.

As far as I can tell, "STDC FENV_ACCESS" and "STDC FENV_ROUND" don't directly 
interact.  FENV_ROUND just overrides the rounding mode for specific 
floating-point operations; it doesn't impact whether environment access is 
allowed.  So the call to setRoundingModeOverride seems dubious.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126364

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

[PATCH] D126364: Fix interaction of pragma FENV_ACCESS with other pragmas

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff created this revision.
sepavloff added reviewers: aaron.ballman, andrew.w.kaylor, rjmccall, efriedma, 
kpn.
Herald added a project: All.
sepavloff requested review of this revision.
Herald added a project: clang.

Previously `#pragma STDC FENV_ACCESS ON` always set dynamic rounding
mode and strict exception handling. It is however too pessimistic in
many cases. In practice the pragma can be used to change rounding mode
only or to check exception flags without changing rounding mode. It
could be made by using other pragmas, `FENV_ROUND` or `clang fp
exception`, but their effect was overwritten by `FENV_ACCESS`. This
change modifies treatment of this pragma so that it sets rounding and
exception handling only if they are not set by other pragmas.

Also `#pragma STDC FENV_ACCESS OFF` turned off only FEnvAccess flag
leaving rounding mode and exception handling unchanged, which does not
make sense in many cases. The change fixes this behavior also.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D126364

Files:
  clang/lib/Sema/SemaAttr.cpp
  clang/test/CodeGen/pragma-fenv_access.c

Index: clang/test/CodeGen/pragma-fenv_access.c
===
--- clang/test/CodeGen/pragma-fenv_access.c
+++ clang/test/CodeGen/pragma-fenv_access.c
@@ -20,7 +20,7 @@
   return x + y;
 }
 // CHECK-LABEL: @func_02
-// CHECK: call float @llvm.experimental.constrained.fadd.f32(float {{.*}}, float {{.*}}, metadata !"round.dynamic", metadata !"fpexcept.ignore")
+// CHECK: call float @llvm.experimental.constrained.fadd.f32(float {{.*}}, float {{.*}}, metadata !"round.tonearest", metadata !"fpexcept.ignore")
 
 
 float func_03(float x, float y) {
@@ -41,7 +41,7 @@
   return x + y;
 }
 // CHECK-LABEL: @func_04
-// CHECK: call float @llvm.experimental.constrained.fadd.f32(float {{.*}}, float {{.*}}, metadata !"round.dynamic", metadata !"fpexcept.ignore")
+// CHECK: fadd float
 
 
 float func_05(float x, float y) {
@@ -57,7 +57,7 @@
   return x + y;
 }
 // CHECK-LABEL: @func_06
-// CHECK: call float @llvm.experimental.constrained.fadd.f32(float {{.*}}, float {{.*}}, metadata !"round.dynamic", metadata !"fpexcept.ignore")
+// CHECK: fadd float
 
 
 float func_07(float x, float y) {
@@ -69,6 +69,58 @@
   return y + 4;
 }
 // CHECK-LABEL: @func_07
-// CHECK: call float @llvm.experimental.constrained.fsub.f32(float {{.*}}, float {{.*}}, metadata !"round.dynamic", metadata !"fpexcept.strict")
+// CHECK: call float @llvm.experimental.constrained.fsub.f32(float {{.*}}, float {{.*}}, metadata !"round.tonearest", metadata !"fpexcept.ignore")
 // CHECK: call float @llvm.experimental.constrained.fmul.f32(float {{.*}}, float {{.*}}, metadata !"round.dynamic", metadata !"fpexcept.strict")
-// CHECK: call float @llvm.experimental.constrained.fadd.f32(float {{.*}}, float {{.*}}, metadata !"round.dynamic", metadata !"fpexcept.strict")
+// CHECK: call float @llvm.experimental.constrained.fadd.f32(float {{.*}}, float {{.*}}, metadata !"round.tonearest", metadata !"fpexcept.ignore")
+
+
+float func_08(float x, float y) {
+  #pragma STDC FENV_ROUND FE_UPWARD
+  #pragma STDC FENV_ACCESS ON
+  return x + y;
+}
+// CHECK-LABEL: @func_08
+// CHECK:  call float @llvm.experimental.constrained.fadd.f32({{.*}}, metadata !"round.upward", metadata !"fpexcept.strict")
+
+
+float func_09(float x, float y) {
+  #pragma clang fp exceptions(maytrap)
+  #pragma STDC FENV_ACCESS ON
+  return x + y;
+}
+// CHECK-LABEL: @func_09
+// CHECK:  call float @llvm.experimental.constrained.fadd.f32({{.*}}, metadata !"round.dynamic", metadata !"fpexcept.maytrap")
+
+float func_10(float x, float y) {
+  #pragma clang fp exceptions(maytrap)
+  #pragma STDC FENV_ROUND FE_UPWARD
+  #pragma STDC FENV_ACCESS ON
+  return x + y;
+}
+// CHECK-LABEL: @func_10
+// CHECK:  call float @llvm.experimental.constrained.fadd.f32({{.*}}, metadata !"round.upward", metadata !"fpexcept.maytrap")
+
+float func_11(float x, float y, float z) {
+  #pragma STDC FENV_ACCESS ON
+  float res = x * y;
+  {
+#pragma STDC FENV_ACCESS OFF
+return res + z;
+  }
+}
+// CHECK-LABEL: @func_11
+// CHECK:  call float @llvm.experimental.constrained.fmul.f32({{.*}}, metadata !"round.dynamic", metadata !"fpexcept.strict")
+// CHECK:  call float @llvm.experimental.constrained.fadd.f32({{.*}}, metadata !"round.tonearest", metadata !"fpexcept.ignore")
+
+float func_12(float x, float y, float z) {
+  #pragma STDC FENV_ROUND FE_TOWARDZERO
+  #pragma STDC FENV_ACCESS ON
+  float res = x * y;
+  {
+#pragma STDC FENV_ACCESS OFF
+return res + z;
+  }
+}
+// CHECK-LABEL: @func_12
+// CHECK:  call float @llvm.experimental.constrained.fmul.f32({{.*}}, metadata !"round.towardzero", metadata !"fpexcept.strict")
+// CHECK:  call float @llvm.experimental.constrained.fadd.f32({{.*}}, metadata !"round.towardzero", metadata !"fpexcept.ignore")
Index: clang/lib/Sema/SemaAttr.cpp
===
--- clan