Set rounding_mode to tonearest in presence of a #pragma STDC FENV_ACCESS OFF.

[zahiraam]

Summary

In strict mode the 'roundin_mode' is set to 'dynamic'. Using this pragma to get out of strict mode doesn't have any effect on the 'rounding_mode'.
See https://godbolt.org/z/zoGTf4j1G
This patch fixes that.

[pengfei]

Remove the fenv_04_06.patch

[pengfei]

clang/test/CodeGen/pragma-fenv_access.c

243

Should this be ignore?

[andrew.w.kaylor]

clang/lib/Sema/SemaAttr.cpp

1340

Why is this only needed for "!IsEnabled"? Where is the rounding mode set in the IsEnabled case? It looks like setAllowFEnvAccessOverride() is defined by a macro and just sets a bit in the OverrideMask, but somehow it seems to be setting the rounding mode to Round.Dynamic. I'm just concerned that there is a disconnect in the implementation here.

[andrew.w.kaylor]

clang/test/CodeGen/pragma-fenv_access.c

243

This is a tricky case. By a strict reading of the C standard, this could be ignore, because the standard says the compiler can assume the default floating point environment when FENV_ACCESS is OFF and that if code compiled with FENV_ACCESS OFF is executed with anything other than the default environment the behavior is undefined. However, in this case strict exception semantics have been enabled elsewhere in the compilation unit, so floating point exceptions may be unmasked. The standard allows us to ignore exceptions, but raising a spurious exception may be bad for users.

I'm unsure about this case. I lean towards leaving it as Zahira has it here because I don't think the use of strict exception semantics will be common enough to justify the less conservative behavior.

[zahiraam]

clang/lib/Sema/SemaAttr.cpp

1340

That all depends on if we are using the -frounding-math in the command line option or not. The LIT test that exercices the pragma is not using this option so it's actually not testing the pragma in a strict mode. Using -ffp-model=strict trigger -frounding-math and -ffp-exception-behavior=strict.

I think I want to make this change only when we are in real strict mode. Meaning we need both of these options to be set.
#pragma STDC FENV_ACCESS OFF should trigger the round.tonearest when both these options are used. So when IsEnabled is false and RoundingMath is 1.

[pengfei]

The change makes more sense to me, thanks!

clang/test/CodeGen/pragma-fenv_access.c

243

Maybe use float %0, float %1 to reduce unnecessary differences.

[zahiraam]

clang/test/CodeGen/pragma-fenv_access.c

243

Andy has noticed that the rest of the test is using the %, so to stay consistent I have put back the %.

[andrew.w.kaylor]

clang/test/CodeGen/pragma-fenv_access.c

4

Why doesn't this case use STRICT-RND? round.tonearest, but I don't understand why.

[zahiraam]

clang/test/CodeGen/pragma-fenv_access.c

4

It does. I just forgot to add the ":" after the RUN.

[andrew.w.kaylor]

Looks good to me.

clang/lib/Sema/SemaAttr.cpp

if (IsEnabled) {
     // Verify Microsoft restriction:
     // You can't enable fenv_access unless precise semantics are enabled.
     // Precise semantics can be enabled either by the float_control
     // pragma, or by using the /fp:precise or /fp:strict compiler options
     if (!isPreciseFPEnabled())
       Diag(Loc, diag::err_pragma_fenv_requires_precise);
   }
   NewFPFeatures.setAllowFEnvAccessOverride(IsEnabled);
+  NewFPFeatures.setRoundingMathOverride(IsEnabled);
   FpPragmaStack.Act(Loc, PSK_Set, StringRef(), NewFPFeatures);
   CurFPFeatures = NewFPFeatures.applyOverrides(getLangOpts());
 }
 
 void Sema::ActOnPragmaFPExceptions(SourceLocation Loc,
                                    LangOptions::FPExceptionModeKind FPE) {
   setExceptionMode(Loc, FPE);
 }
 

clang/test/CodeGen/pragma-fenv_access.c

 // 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 -frounding-math -ffp-exception-behavior=strict -triple %itanium_abi_triple -emit-llvm %s -o - | FileCheck --check-prefixes=CHECK,STRICT-RND %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 -frounding-math -ffp-exception-behavior=strict -triple %itanium_abi_triple -emit-llvm %s -o - -fms-extensions -DMS | FileCheck --check-prefixes=CHECK,STRICT-RND %s
 // RUN: %clang_cc1 -fexperimental-strict-floating-point -triple %itanium_abi_triple -emit-llvm %s -o - | FileCheck --check-prefixes=CHECK,DEFAULT %s
-
+// RUN: %clang_cc1 -fexperimental-strict-floating-point -frounding-math -triple %itanium_abi_triple -emit-llvm %s -o - | FileCheck --check-prefixes=CHECK,DEFAULT-RND %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")
+// STRICT-RND: call float @llvm.experimental.constrained.fadd.f32(float {{.*}}, float {{.*}}, metadata !"round.dynamic", metadata !"fpexcept.strict")
+// DEFAULT-RND: call float @llvm.experimental.constrained.fadd.f32(float {{.*}}, float {{.*}}, metadata !"round.dynamic", metadata !"fpexcept.ignore")
 // DEFAULT: fadd float
 
 
 #ifdef MS
 #pragma fenv_access (on)
 #else
 #pragma STDC FENV_ACCESS ON
 #endif

 float func_18(float x, float y) {
   #pragma STDC FENV_ROUND FE_DYNAMIC
   return x + y;
 }
 // CHECK-LABEL: @func_18
 // STRICT: call float @llvm.experimental.constrained.fadd.f32(float {{.*}}, float {{.*}}, metadata !"round.tonearest", metadata !"fpexcept.strict")
 // DEFAULT: fadd float
 
+#pragma STDC FENV_ACCESS ON
+float func_19(float x, float y) {
+  return x + y;
+}
+// CHECK-LABEL: @func_19
+// STRICT:  call float @llvm.experimental.constrained.fadd.f32(float {{.*}}, float {{.*}}, metadata !"round.dynamic", metadata !"fpexcept.strict")
+
+#pragma STDC FENV_ACCESS OFF
+float func_20(float x, float y) {
+  return x + y;
+}
+// CHECK-LABEL: @func_20
+// STRICT: call float @llvm.experimental.constrained.fadd.f32(float {{.*}}, float {{.*}}, metadata !"round.tonearest", metadata !"fpexcept.strict")
+// DEFAULT: fadd float