michele.scandale added inline comments.
================ 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: > 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. Prior to D72841 compiling something with `-ffast-math -ffp-contract={on,off}` was still producing `fast` as fast-math flags on the instructions for the same reason. The clang driver does not consider the contraction mode for passing `-ffast-math` to CC1, which is consistent with the GCC behavior (I checked if the `__FAST_MATH__` is defined if I compile with `-ffast-math -ffp-contract=off`). According to this, the code in `CodeGenFunction`: ``` if (LangOpts.FastMath) FMF.setFast(); ``` is not correct. The OpenCL option `-cl-fast-relaxed-math` should be equivalent to `-ffast-math` for the user. I don't know what the interaction between `-cl-fast-relaxed-math` and `-ffp-contract={on, off,fast}`. If we need to keep `-cl-fast-relaxed-math` a CC1 option, I guess the following might work: ``` if (Args.hasArg(OPTS_cl_fast_relaxed_math) && !Arg.hasArg(OPT_ffp_contractEQ)) Opts.setDefaultFPContractMode)LangOptions::FPM_Fast); ``` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79903/new/ https://reviews.llvm.org/D79903 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
