[PATCH] D72841: [RFC] Add support for pragma float_control, to control precision and exception behavior at the source level

[Melanie Blower via Phabricator via llvm-commits]

mibintc marked 3 inline comments as done.
mibintc added a comment.

In D72841#1833022 <https://reviews.llvm.org/D72841#1833022>, @sepavloff wrote:

> I don't see tests for correctness of the pragma stack (`pragma float_control(... push)`, `pragma float_control(pop)`). Can you add them?


I added a test case for the stack of float_control, and it's also hitting the problem with the function scope not being closed by the right brace. This patch is useless until I tackle that problem.  In the test case I put in bogus declarations which force the function scope to close.  There are some pragmas called "range pragmas" that affect the functions in a source location range in the file. I don't know if I could mix that idea with the MS pragma stack that I'm using to push/pop the pragma settings, I'm doubtful.



================
Comment at: clang/lib/Sema/SemaExpr.cpp:13129
     if (FunctionDecl *F = dyn_cast<FunctionDecl>(CurContext)) {
+      // If the expression occurs inside an internal global_var_init_function
+      // then the FunctionDecl is not availble
----------------
mibintc wrote:
> sepavloff wrote:
> > mibintc wrote:
> > > sepavloff wrote:
> > > > mibintc wrote:
> > > > > sepavloff wrote:
> > > > > > The standard says that static initializers execute in default FP mode.
> > > > > > The standard says ...
> > > > > Are you sure about this one?  Can you please provide the standards reference so I can study it?
> > > > >> The standard says ...
> > > > > Are you sure about this one? Can you please provide the standards reference so I can study it?
> > > > 
> > > > http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1570.pdf, F.8.5:
> > > > ```
> > > > ... All computation for initialization of objects that have static or thread storage duration is done (as if) at translation time.
> > > > ```
> > > > F.8.2:
> > > > ```
> > > > During translation the IEC 60559 default modes are in effect:
> > > > — The rounding direction mode is rounding to nearest.
> > > > — The rounding precision mode (if supported) is set so that results are not shortened.
> > > > — Trapping or stopping (if supported) is disabled on all floating-point exceptions.
> > > > ```
> > > Thanks for the pointer to the reference. The desired semantics of the pragma may differ from the standard. For example I tried this test case with the fp_contract pragma, and the pragma does modify the semantics of the floating point expressions in the initializer.  So this issue is still a problem in this patch. 
> > > 
> > > // RUN: %clang_cc1 -emit-llvm -o - %s | FileCheck %s
> > > 
> > > #pragma STDC FP_CONTRACT ON
> > > float y();
> > > float d();
> > > class ON {
> > > float z = y() * 1 + d();
> > > // CHECK-LABEL: define {{.*}} void @_ZN2ONC2Ev{{.*}}
> > > //CHECK: llvm.fmuladd.f32{{.*}}
> > > };
> > > ON on;
> > > 
> > > #pragma STDC FP_CONTRACT OFF
> > > class OFF {
> > > float w = y() * 1 + d();
> > > // CHECK-LABEL: define {{.*}} void @_ZN3OFFC2Ev{{.*}}
> > > //CHECK: fmul float
> > > };
> > > OFF off;
> > > 
> > This is an interesting example. However there is no contradiction with the standard. The standard speaks about floating point environment, which on most processors are represented by bits of some register(s). The pragma `STDC FP_CONTRACT` does not refer to the FPEnv, but is an instruction to the compiler how to generate code, so it affects code generation even in global var initializers.
> > 
> > What `TBD` here means? Do you think this code may be somehow improved?
> I wasn't yet certain about the interpretation of the pragma on the initializatin expressions. Today I did some testing with ICL and CL and it seems the pragma has no effect on the initialization expressions that occur within constructors in classes at file scope.  So I'll remove the TBD and the current behavior in this patch wrt this question is OK.
I removed the TBD, thanks. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72841