yaxunl added inline comments.
================ Comment at: clang/lib/CodeGen/BackendUtil.cpp:486 + if (LangOpts.HIP) + Options.AllowFPOpFusion = llvm::FPOpFusion::Standard; + ---------------- rjmccall wrote: > 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. I think probably we need to introduce a new value for `-ffp-contract=` option `faststd`, which is `fast` for FE and `Standard` for BE. In this case, fp add/mult by default can be fused, unless disabled by pragma, and BE respect the restrictions imposed by pragmas in FE. I think this mode is more useful than the original `fast` mode. 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
