[PATCH] D87528: Enable '#pragma STDC FENV_ACCESS' in frontend cf. D69272 - Work in Progress

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff added inline comments.


================
Comment at: clang/lib/Sema/SemaAttr.cpp:1020-1023
+    // Resume the default rounding and exception modes.
+    NewFPFeatures.setRoundingModeOverride(
+        llvm::RoundingMode::NearestTiesToEven);
+    NewFPFeatures.setFPExceptionModeOverride(LangOptions::FPE_Ignore);
----------------
mibintc wrote:
> sepavloff wrote:
> > The previous values of rounding mode and exception behavior may differ from the default values. For example, `#pragma STDC FENV_ROUND` may set constant rounding direction different from FE_TONEAREST`. Similarly, command line options may set exception behavior different from `ignore`.
> > Can pragma stack be used for this?
> > The previous values of rounding mode and exception behavior may differ from the default values. For example, `#pragma STDC FENV_ROUND` may set constant rounding direction different from FE_TONEAREST`. Similarly, command line options may set exception behavior different from `ignore`.
> > Can pragma stack be used for this?
> 
> I guess I could just NewFPFeatures.setAllowFEnvAccessOverride(false); and leave the other 2 unset, does that sound right? The values of the FPFeatures are being preserved around compound statements with FPFeaturesStateRAII
> I guess I could just NewFPFeatures.setAllowFEnvAccessOverride(false); and leave the other 2 unset, does that sound right?

It should fix the most important case:
```
// RUN: %clang_cc1 -S -emit-llvm %s -o - | FileCheck %s

float func_01(float x, float y, float z) {
  #pragma STDC FENV_ROUND FE_DOWNWARD
  float res = x + y;
  {
    #pragma STDC FENV_ACCESS OFF
    res *= z;
  }
  return res - x;
}

// CHECK: call float @llvm.experimental.constrained.fadd.f32(float {{.*}}, float {{.*}}, metadata !"round.downward", metadata !"fpexcept.ignore")
// CHECK: call float @llvm.experimental.constrained.fmul.f32(float {{.*}}, float {{.*}}, metadata !"round.downward", metadata !"fpexcept.ignore")
// CHECK: call float @llvm.experimental.constrained.fsub.f32(float {{.*}}, float {{.*}}, metadata !"round.downward", metadata !"fpexcept.ignore")
```
Another case, which now fails is:
```
// RUN: %clang_cc1 -S -emit-llvm -frounding-math %s -o - | FileCheck %s

float func_01(float x, float y, float z) {
  float res = x + y;
  {
    #pragma STDC FENV_ACCESS OFF
    res *= z;
  }
  return res - x;
}

// CHECK: call float @llvm.experimental.constrained.fadd.f32(float {{.*}}, float {{.*}}, metadata !"round.dynamic", metadata !"fpexcept.ignore")
// CHECK: call float @llvm.experimental.constrained.fmul.f32(float {{.*}}, float {{.*}}, metadata !"round.dynamic", metadata !"fpexcept.ignore")
// CHECK: call float @llvm.experimental.constrained.fsub.f32(float {{.*}}, float {{.*}}, metadata !"round.dynamic", metadata !"fpexcept.ignore")
```
The function may be called from a context, where rounding mode is not default, command-line options allow that. So rounding mode inside the inner block is dynamic, although FP state is not accessed in it.

Now consider exception behavior. For example, the test case, which now passes:
```
// RUN: %clang_cc1 -S -emit-llvm -ffp-exception-behavior=strict %s -o - | FileCheck %s

float func_01(float x, float y, float z) {
  float res = x + y;
  {
    #pragma STDC FENV_ACCESS OFF
    res *= z;
  }
  return res - x;
}

// CHECK: call float @llvm.experimental.constrained.fadd.f32(float {{.*}}, float {{.*}}, metadata !"round.tonearest", metadata !"fpexcept.strict")
// CHECK: call float @llvm.experimental.constrained.fmul.f32(float {{.*}}, float {{.*}}, metadata !"round.tonearest", metadata !"fpexcept.ignore")
// CHECK: call float @llvm.experimental.constrained.fsub.f32(float {{.*}}, float {{.*}}, metadata !"round.tonearest", metadata !"fpexcept.strict")
```
... is correct IMHO. Setting FENV_ACCESS to OFF means the code in the affected region does not check FP exceptions, so they can be ignored. This is a way to have default FP state even if the command-line options set FENV_ACCESS. As for `maytrap` I am not sure, but I think the following test should pass:  
```
// RUN: %clang_cc1 -S -emit-llvm -ffp-exception-behavior=maytrap %s -o - | FileCheck %s

float func_01(float x, float y, float z) {
  float res = x + y;
  {
    #pragma STDC FENV_ACCESS OFF
    res *= z;
  }
  return res - x;
}

// CHECK: call float @llvm.experimental.constrained.fadd.f32(float {{.*}}, float {{.*}}, metadata !"round.tonearest", metadata !"fpexcept.maytrap")
// CHECK: call float @llvm.experimental.constrained.fmul.f32(float {{.*}}, float {{.*}}, metadata !"round.tonearest", metadata !"fpexcept.maytrap")
// CHECK: call float @llvm.experimental.constrained.fsub.f32(float {{.*}}, float {{.*}}, metadata !"round.tonearest", metadata !"fpexcept.maytrap")
```
The outer code may set hardware for trapping and although in the inner block there is no access to FP state, traps are possible and compiler must be aware of them.

And finally the following test case should work:
```
// RUN: %clang_cc1 -S -emit-llvm %s -o - | FileCheck %s

float func_01(float x, float y, float z) {
  #pragma STDC FENV_ACCESS ON
  float res = x + y;
  {
    #pragma STDC FENV_ACCESS OFF
    res *= z;
  }
  return res - x;
}

// CHECK: call float @llvm.experimental.constrained.fadd.f32(float {{.*}}, float {{.*}}, metadata !"round.dynamic", metadata !"fpexcept.strict")
// CHECK: call float @llvm.experimental.constrained.fmul.f32(float {{.*}}, float {{.*}}, metadata !"round.dynamic", metadata !"fpexcept.ignore")
// CHECK: call float @llvm.experimental.constrained.fsub.f32(float {{.*}}, float {{.*}}, metadata !"round.dynamic", metadata !"fpexcept.strict")
```
Again, the pragma in the inner block claims there is no access to FP environment, but outer block can have set non-default rounding mode.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87528