sepavloff added a comment.

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



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


================
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:
> > > > 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?


================
Comment at: clang/test/CodeGen/fp-floatcontrol-pragma.cpp:2
+// RUN: %clang_cc1 -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -fsyntax-only -verify -DCHECK_ERROR %s
+
----------------
You need to extract the tests that check error generation from this file and 
put them into `clang/test/Parser`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72841



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

Reply via email to