rjmccall added inline comments.
================ Comment at: clang/lib/CodeGen/BackendUtil.cpp:486 + if (LangOpts.HIP) + Options.AllowFPOpFusion = llvm::FPOpFusion::Standard; + ---------------- tra wrote: > yaxunl wrote: > > tra wrote: > > > yaxunl wrote: > > > > tra wrote: > > > > > I don't think it's a good idea to force this. > > > > > > > > > > Perhaps a better way to address this would be to set HIP-specific > > > > > default to Standard where CUDA does it: > > > > > https://github.com/llvm/llvm-project/blob/master/clang/lib/Frontend/CompilerInvocation.cpp#L2415 > > > > > > > > > > Currently HIP inherits this setting from CUDA. > > > > We want to keep -ffp-contract=fast for frontend so that we can continue > > > > emitting fmul/fadd insts with contract flag in IR for HIP programs. We > > > > only want to change the backend fp fuse option. Currently there is no > > > > separate clang option to set backend fp fuse option. > > > I do not see any references to `AllowFPOpFusion` anywhere under `clang/` > > > other than in this function. > > > Perhaps I'm missing something. How/where does it make a difference in the > > > front-end other than setting the option for the back-end? > > > > > > > > > > > -ffp-contract not only sets backend fp fuse option, but also > > setFPContractMode > > > > https://github.com/llvm/llvm-project/blob/2e204e23911b1f8bd1463535da40c6e48747a138/clang/include/clang/Basic/LangOptions.h#L413 > > > > which enables allowFPContractAcrossStatement > > > > https://github.com/llvm/llvm-project/blob/2e204e23911b1f8bd1463535da40c6e48747a138/clang/include/clang/Basic/LangOptions.h#L439 > > > > which sets llvm::FastMathFlags > > > > https://github.com/llvm/llvm-project/blob/d3205bbca3e0002d76282878986993e7e7994779/clang/lib/CodeGen/CodeGenFunction.cpp#L129 > > > > which causes fmul/fadd to have contract flag > > > > > > > > > > > > > Thank you. I see. > I'm still uncomfortable with growing a target-specific quirk in FP behavior > that can't be overridden from command line, but considering that it's limted > to HIP only, it may be OK short-term. > > I think similar issue (fast-math does not allow changing contraction mode) > was discussed last year: > http://lists.llvm.org/pipermail/llvm-dev/2019-July/133787.html > > One thing I know about FP is that there are more nuances than I'm aware of. > It may be worth asking with more experience with this. > > @scanon -- do we have any better options to honor contraction pragmas with > **implicitly**-enabled fast-math? > In the abstract, it seems reasonable for pragmas to override a global fast-math setting. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90174/new/ https://reviews.llvm.org/D90174 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
