[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 2 inline comments as done.
mibintc added a comment.

A couple inline replies to @sepavloff ; I'll be uploading another revision.



================
Comment at: clang/lib/Parse/ParsePragma.cpp:2537
+    if (!Actions.CurContext->isTranslationUnit()) {
+//FIXME this seems to be the wrong way to check file-scope
+//since the token immediately following a function definition
----------------
sepavloff wrote:
> mibintc wrote:
> > sepavloff wrote:
> > > Probably using `Actions.getCurScope()` can help to recognize file scope.
> > Thanks for the suggestion, I (Actions.getCurScope()==0) to test for file scope, but that didn't work either. I put a workaround into the test case CodeGen/fp-floatcontrol-pragma.cpp, the forward class declaration ResetTUScope. If the reset is there, then the pragma is recognized to be at file scope. 
> `Scope` always exists, so the correct way to check if it refers to translation unit is something like: `Actions.getCurScope()->getParent() == nullptr`.
I tried this also: (Actions.getCurScope()->getParent() != nullptr) but that also failed to detect current scope is file scope. In the debugger, where the pragma occurs immediately after a function definition, I can see that CurScope is still the function body. The transition to outer scope must not yet have occurred. I can investigate. 


================
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
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72841