[PATCH] D79903: FastMathFlags.allowContract should be init from FPFeatures.allowFPContractAcrossStatement

[Melanie Blower via Phabricator via cfe-commits]

mibintc marked an inline comment as done.
mibintc added a comment.

reply about the incorrect setting of 'fast' during OpenCL compilation with option -cl-fast-relaxed-math



================
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:2943
+  if (Opts.FastRelaxedMath)
+    Opts.setDefaultFPContractMode(LangOptions::FPM_Fast);
   Opts.HexagonQdsp6Compat = Args.hasArg(OPT_mqdsp6_compat);
----------------
mibintc wrote:
> rjmccall wrote:
> > mibintc wrote:
> > > rjmccall wrote:
> > > > mibintc wrote:
> > > > > I changed this because the FAST version of this test clang/test/CodeGenOpenCL/relaxed-fpmath.cl wants the 'fast' attribute on the instruction dump.  All the LLVM FMF bits must be set for the fast attribute print.  By default, the value for OpenCL is ffp-contract=on
> > > > Is there an overall behavior change for OpenCL across these patches?
> > > I think the answer to your question is no.  Here is more details: OpenCL sets the default behavior to ffp-contract=on.  In https://reviews.llvm.org/D72841 I added the function UpdateFastMathFlags, that function set the llvm FMF.allowContract bit to be ( ffp-contract=on or ffp-contract=fast).  This patch just drops the check on ffp-contract=on.   I didn't wind back to see how the llvm fast attribute was being set for this [opencl] test case originally. 
> > Well, to what extent are there (including this patch) overall test changes for the OpenCL tests, and what are tthey?
> In the #pragma float_control patch https://reviews.llvm.org/D72841, I changed 2 CodeGen/OpenCL tests: relaxed-fp-math.cl in the non-FAST cases to show the contract bit.  Also 1 line in single-precision-constant.cl for the same reason, to show the contract bit.  This patch undoes those test changes. I'll do more investigation to understand why the fast bit isn't being set in the FAST case in relaxed-fpmath.cl without the change to CompilerInovcaton
Prior to the patch for #pragma float_control, the llvm.FastMathFlags was initialized from LangArgs.FastMath in the CodeGenFunction constructor approximately line 74, and the FMF values in IRBuilder were never changed--For the test clang/test/CodeGenOpenCL/relaxed-fpmath.cl with option -cl-fast-relaxed-math.  (In ParseLangArgs,  Opts.FastMath = Args.hasArg(OPT_ffast_math) ||      Args.hasArg(OPT_cl_fast_relaxed_math))  If FastMath is on, then all the llvm.FMF flags are set.

The #pragma float_control patch does change the IRBuilder settings in the course of visiting the Expr nodes, using the information in the Expr nodes, but the initial setting of FPFeatures from the command line overlooked the fact that FastMath, and therefore ffp-contract=fast, is enabled. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79903