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

mibintc added a comment. Herald added subscribers: jansvoboda11, dexonsmith. I got a bug report about this patch, see https://bugs.llvm.org/show_bug.cgi?id=49479. I put a patch to fix it here, https://reviews.llvm.org/D98211 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

cfang added a comment. -ffast-math flag got lost in the Builder after this change. FMF.isFast() is true before updateFastMathFlags(FMF, FPFeatures), but turns false after. It seems the Builder.FMF has been correctly set before, but I am not clear what FPFeatures should be at this point:

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

mibintc added a comment. Herald added a subscriber: sstefan1. I documented the issue reported by @yaxunl here, https://bugs.llvm.org/show_bug.cgi?id=46166, and take ownership of the bug. Thanks for the report. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

michele.scandale added inline comments. Comment at: clang/lib/CodeGen/CGExprScalar.cpp:227 + FMF.setAllowContract(FPFeatures.allowFPContractAcrossStatement() || + FPFeatures.allowFPContractWithinStatement()); } mibintc wrote: > mibintc

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

rjmccall added inline comments. Comment at: clang/lib/Serialization/ASTReader.cpp:7899 +if (FpPragmaCurrentLocation.isInvalid()) { + assert(*FpPragmaCurrentValue == SemaObj->FpPragmaStack.DefaultValue && + "Expected a default pragma float_control value");

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

mibintc marked 2 inline comments as done. mibintc added inline comments. Comment at: clang/lib/CodeGen/CGExprScalar.cpp:227 + FMF.setAllowContract(FPFeatures.allowFPContractAcrossStatement() || + FPFeatures.allowFPContractWithinStatement()); }

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

yaxunl added inline comments. Comment at: clang/lib/Serialization/ASTReader.cpp:7899 +if (FpPragmaCurrentLocation.isInvalid()) { + assert(*FpPragmaCurrentValue == SemaObj->FpPragmaStack.DefaultValue && + "Expected a default pragma float_control value");

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

mibintc marked an inline comment as done. mibintc added inline comments. Comment at: clang/lib/Serialization/ASTReader.cpp:7899 +if (FpPragmaCurrentLocation.isInvalid()) { + assert(*FpPragmaCurrentValue == SemaObj->FpPragmaStack.DefaultValue && + "Expected a

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

yaxunl added inline comments. Comment at: clang/lib/Serialization/ASTReader.cpp:7899 +if (FpPragmaCurrentLocation.isInvalid()) { + assert(*FpPragmaCurrentValue == SemaObj->FpPragmaStack.DefaultValue && + "Expected a default pragma float_control value");

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

mibintc added inline comments. Comment at: clang/lib/CodeGen/CGExprScalar.cpp:227 + FMF.setAllowContract(FPFeatures.allowFPContractAcrossStatement() || + FPFeatures.allowFPContractWithinStatement()); } michele.scandale wrote: > I'm not

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

scanon added a comment. > Prior to this change contract was never generated in the case of in-statement > contraction only, instead clang was emitting llvm.fmuladd to inform the > backend that only those were eligible for contraction. From a correctness > perspective I think this was perfectly

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

rjmccall added a comment. No, if that's how we handle that, then you're absolutely right that we shouldn't set the `contract` flag. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72841/new/ https://reviews.llvm.org/D72841

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

michele.scandale added a comment. In D72841#2030707 , @rjmccall wrote: > IIUC, the way within-statement contraction is supposed to work is that there > are supposed to be blocking intrinsics inserted at various places. I don't > remember the details,

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

michele.scandale added inline comments. Comment at: clang/include/clang/Basic/LangOptions.h:396-401 +allow_reassoc(LangOpts.FastMath || LangOpts.AllowFPReassoc), +no_nans(LangOpts.FastMath || LangOpts.NoHonorNaNs), +no_infs(LangOpts.FastMath ||

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

rjmccall added a comment. IIUC, the way within-statement contraction is supposed to work is that there are supposed to be blocking intrinsics inserted at various places. I don't remember the details, though, or if anyone's thought about how it interacts with cross-statement contraction, which

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

michele.scandale added inline comments. Comment at: clang/lib/CodeGen/CGExprScalar.cpp:227 + FMF.setAllowContract(FPFeatures.allowFPContractAcrossStatement() || + FPFeatures.allowFPContractWithinStatement()); } I'm not convinced it

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

rjmccall added a comment. I think we might have had to change that test on our fork when we changed the parsing order. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72841/new/ https://reviews.llvm.org/D72841

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

plotfi added a comment. @rjmccall @mibintc I think the diagnostics-order.c test is still behaving correctly technically. The note lines are still printing with the associated error lines, it just happens that one of the warning lines prints at the end instead of in the middle. ie: error:

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

plotfi added a comment. That was a good fix. I am pretty sure this does mean the diagnostics-order.c will fail on apple's bots. The same diagnostics lines print, but in the wrong order. I haven't root caused that yet. In D72841#2030099 , @mibintc wrote:

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

mibintc added a comment. In D72841#2029821 , @plotfi wrote: > @ab @rjmccall @mibintc Posted D79730 for > consideration. > @mibintc can you produce a version of _this_ diff that works with D79730 >

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

plotfi added a subscriber: ab. plotfi added a comment. @ab @rjmccall @mibintc Posted D79730 for consideration. @mibintc can you produce a version of _this_ diff that works with D79730 applied. Currently the following fail, as

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

rjmccall added inline comments. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3192 + Opts.NoHonorNaNs = + Opts.FastMath || CGOpts.NoNaNsFPMath || Opts.FiniteMathOnly; + Opts.NoHonorInfs = mibintc wrote: > @rjmccall I could set these by using

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

rjmccall added inline comments. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3185 Opts.FiniteMathOnly = Args.hasArg(OPT_ffinite_math_only) || Args.hasArg(OPT_cl_finite_math_only) || Args.hasArg(OPT_cl_fast_relaxed_math); mibintc wrote:

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

mibintc added inline comments. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3192 + Opts.NoHonorNaNs = + Opts.FastMath || CGOpts.NoNaNsFPMath || Opts.FiniteMathOnly; + Opts.NoHonorInfs = @rjmccall I could set these by using Args.hasArg instead of

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

mibintc added a comment. @rjmccall Uncertain how to proceed, can you recommend? If I recall correctly, I added the lines in CompilerOptions because there were many failing lit tests, i could have fixed the lit fails by adding the lang options to the lit tests. (of course that change could

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

mibintc marked 2 inline comments as done. mibintc added a comment. Some inline replies/comments to @rjmccall and @plotfi Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3185 Opts.FiniteMathOnly = Args.hasArg(OPT_ffinite_math_only) ||

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

mibintc added a comment. I will work @rjmccall comment about codegen vs langopt - can you leave it in place for now? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72841/new/ https://reviews.llvm.org/D72841

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

plotfi added a comment. @rjmccall @mibintc Can we revert this patch for now then, and re-land when this patch is reworked? It would be good to get those bots passing. @rjmccall are the bots that you see failing on your end public? In D72841#2028834 ,

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

rjmccall added a comment. In D72841#2027740 , @plotfi wrote: > Hi @rjmccall, I am also seeing similar failures. It is failing on apple's > master (and the swift branches as well) because ParseLangArgs and > ParseCodeGenArgs are getting called in the

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

plotfi added a comment. Hi @rjmccall, I am also seeing similar failures. It is failing on apple's master (and the swift branches as well) because ParseLangArgs and ParseCodeGenArgs are getting called in the opposite order in apple/master from the order they are called in llvm/master. I posted

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

mibintc added a comment. I got a report that this patch was causing a problem with Windows header because #pragma float_control should be supported in namespace context. I've posted a patch for review here https://reviews.llvm.org/D79631 Repository: rG LLVM Github Monorepo CHANGES SINCE

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

mibintc added a comment. I posted a patch to fix the bug reported by @uabelho here https://reviews.llvm.org/D79510 @rjmccall I used check-clang and check-all on D71841 from my linux x86-64 server before submitting, and the testing was clear. Maybe your branch

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

rjmccall added a comment. We're also seeing test failures in Apple's Clang fork, e.g. test/CodeGen/finite-math.c:12:10: error: NSZ: expected string not found in input // NSZ: fadd nsz ^ :11:20: note: scanning from here define void @foo() #0 { ^ :15:9:

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

mibintc added a comment. In D72841#2022354 , @uabelho wrote: > It seems to be this change in SemaStmt.cpp that makes the FENV-pragma have > some effect regardless of the warning saying that the pragma doesn't have > any effect: > > index

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

uabelho added a comment. It seems to be this change in SemaStmt.cpp that makes the FENV-pragma have some effect regardless of the warning saying that the pragma doesn't have any effect: index aa0d89ac09c3..f76994a6dab3 100644 @@ -394,6 +394,11 @@ StmtResult

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

uabelho added a comment. Hi @mibintc , I think I'm seeing some oddities with this patch (current trunk, 0054c46095 ). With clang -S -Xclang -emit-llvm bbi-42553.c -o - on the input float rintf(float x); #pragma STDC

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

erichkeane accepted this revision. erichkeane added a comment. This revision is now accepted and ready to land. Looks OK to me. Please give @sepavloff and @rjmccall a day or so to make final comments before committing. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

erichkeane added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:870 +def err_pragma_fenv_requires_precise : Error< + "'#pragma STDC FENV_ACCESS ON' is illegal when precise is disabled">; def warn_cxx_ms_struct : mibintc wrote: >

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

mibintc marked an inline comment as done. mibintc added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:870 +def err_pragma_fenv_requires_precise : Error< + "'#pragma STDC FENV_ACCESS ON' is illegal when precise is disabled">; def

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

erichkeane added a comment. 2 small things, @rjmccall and @sepavloff , anything else? Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:870 +def err_pragma_fenv_requires_precise : Error< + "'#pragma STDC FENV_ACCESS ON' is illegal when precise is disabled">; def

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

mibintc marked an inline comment as done. mibintc added inline comments. Comment at: clang/include/clang/AST/Expr.h:2701 + FPOptions FPFeatures; + mibintc wrote: > erichkeane wrote: > > This type already has trailing-storage type stuff. I think in the past

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

mibintc marked 5 inline comments as done. mibintc added a comment. A couple replies to @erichkeane Comment at: clang/include/clang/AST/Expr.h:2251 + /// allocated in Trailing Storage + void setHasStoredFPFeatures(bool B) { UnaryOperatorBits.HasFPFeatures = B; } + bool

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

erichkeane added a comment. @rjmccall : can you comment on the CallExpr's trailing storage issue? And give advice as to what you meant in the last review? Comment at: clang/include/clang/AST/Expr.h:2122 +return *getTrailingObjects(); + } + const FPOptions () const {

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

mibintc marked an inline comment as done. mibintc added inline comments. Comment at: clang/docs/LanguageExtensions.rst:3197 +by the pragma behaves as though the command-line option + ``-ffp-exception-behavior=strict`` is enabled, +when ``pragma float_control(precise, off)`` is

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

mibintc added a comment. noted bug in an inline comment. i will upload a fix Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72841/new/ https://reviews.llvm.org/D72841 ___ cfe-commits mailing list

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

mibintc marked 2 inline comments as done. mibintc added a comment. added a couple inline explanatory comments Comment at: clang/include/clang/Basic/LangOptions.h:307 + bool denormalIsIEEE = false; + I added this boolean as part of validating the correctness