mibintc marked 2 inline comments as done. mibintc added a comment. In D87528#2270541 <https://reviews.llvm.org/D87528#2270541>, @rsmith wrote:
>> @rsmith asked "I don't see changes to the constant evaluator". The semantic >> rules for enabling this pragma require that strict or precise semantics be >> in effect., see SemaAttr.cpp And the floating point constant evaluation >> doesn't occur when strict or precise > > What do you mean "the floating point constant evaluation doesn't occur when > strict or precise"? I don't see any code to do that in `ExprConstant.cpp`, > neither in trunk nor in this patch. The constant evaluator certainly is still > invoked when we're in strict or precise mode. I would expect the evaluator > would need to check for FP strictness whenever it encounters a floating-point > operator (eg, here: > https://github.com/llvm/llvm-project/blob/master/clang/lib/AST/ExprConstant.cpp#L13340), > and treat such cases as non-constant. 1.0/3.0 is now static folded. @rsmith Did you change your mind about this? Is static folding what you want here? > I'd like to see a C++ test for something like this: > > float myAdd() { > #pragma STDC FENV_ACCESS ON > static double v = 1.0 / 3.0; > return v; > } > > If that generates a runtime guarded initialization for `v`, then I'd be a bit > more convinced we're getting this right. Some inline comments, still owe you a list of test that fails. check-clang runs clean with this patch ================ Comment at: clang/lib/AST/ExprConstant.cpp:2496-2498 + if (llvm::APFloatBase::opOK != Result.convertFromAPInt(Value, + Value.isSigned(), + APFloat::rmNearestTiesToEven) && ---------------- rsmith wrote: > Following D89360, we should skip this check if `Info.InConstantContext`. I tried making this change but I got about 30 lit fails, maybe my patch was incorrect. I'll rerun with the patch and post the list of fails. ================ Comment at: clang/test/CodeGen/fp-floatcontrol-pragma.cpp:159 + //CHECK: store float 3.0{{.*}}retval{{.*}} + static double v = 1.0 / 3.0; + //CHECK-FENV: llvm.experimental.constrained.fptrunc.f32.f64{{.*}} ---------------- @rsmith - the division is constant folded. is that right? ================ Comment at: clang/test/Parser/pragma-fenv_access.c:28 +#if defined(CPP) & defined(STRICT) +//not-expected-error@+3 {{constexpr variable 'frac' must be initialized by a constant expression}} +//not-expected-note@+2 {{compile time floating point arithmetic suppressed in strict evaluation modes}} ---------------- @rsmith no diagnostic, is this OK? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87528/new/ https://reviews.llvm.org/D87528 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
