mibintc marked 2 inline comments as done. mibintc 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); ---------------- 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 ================ Comment at: clang/lib/Sema/SemaAttr.cpp:460 case PFC_Push: - Action = Sema::PSK_Push_Set; - FpPragmaStack.Act(Loc, Action, StringRef(), NewFPFeatures.getAsOpaqueInt()); + if (FpPragmaStack.Stack.empty()) { + FpPragmaStack.Act(Loc, Sema::PSK_Set, StringRef(), ---------------- rjmccall wrote: > mibintc wrote: > > When i was adding a test, I realized that pragma float_control(push) then > > pop wasn't working as expected. If the stack is empty, which is most of the > > time, first need to push the current fp features onto the stack so they can > > be restored at the pop > It seems that the way `PragmaStack` is supposed to be used is that we're > supposed to be using its `CurrentValue` instead of having a separate > `CurFPFeatures`. But mostly it looks like there's a lot of functionality > associated with `PragmaStack` that we aren't using at all because this is a > substantially simpler mode; we could really just be using a > `SmallVector<FPOptions, 2>`. > > Anyway, I guess this fix works, although it should be done in a separate > patch. I'll submit it in a separate patch. 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
