[PATCH] D69878: Consoldiate internal denormal flushing controls

arsenm closed this revision. arsenm added a comment. a4451d88ee456304c26d552749aea6a7f5154bde CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69878/new/ https://reviews.llvm.org/D69878 __

[PATCH] D69878: Consoldiate internal denormal flushing controls

cameron.mcinally accepted this revision. cameron.mcinally added a comment. LGTM too. Would be good if an expert reviewed the target-specific changes, but they seem safe enough either way. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69878/new/ https://reviews.llvm.org/D69878 __

[PATCH] D69878: Consoldiate internal denormal flushing controls

andrew.w.kaylor accepted this revision. andrew.w.kaylor added a comment. This revision is now accepted and ready to land. I don't know if there were other reviewers who haven't commented on how you addressed their concerns, but this looks good to me. Thanks for taking the time to improve our han

[PATCH] D69878: Consoldiate internal denormal flushing controls

arsenm marked an inline comment as done. arsenm added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2757 // subsequent options conflict then emit warning diagnostic. + // TODO: How should this interact with DenormalFP32Math? if (HonorINFs &

[PATCH] D69878: Consoldiate internal denormal flushing controls

arsenm updated this revision to Diff 238670. arsenm added a comment. Forgot clang parts CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69878/new/ https://reviews.llvm.org/D69878 Files: clang/include/clang/Basic/CodeGenOptions.def clang/include/clang/Basic/CodeGenOptions.h clang/in

[PATCH] D69878: Consoldiate internal denormal flushing controls

andrew.w.kaylor added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2311 + bool TrappingMath = true; // overriden by ffp-exception-behavior? bool RoundingFPMath = false; arsenm wrote: > cameron.mcinall

[PATCH] D69878: Consoldiate internal denormal flushing controls

arsenm added a comment. Herald added a subscriber: kerbowa. ping CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69878/new/ https://reviews.llvm.org/D69878 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin

[PATCH] D69878: Consoldiate internal denormal flushing controls

arsenm updated this revision to Diff 237665. arsenm added a comment. Mention support in langref CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69878/new/ https://reviews.llvm.org/D69878 Files: clang/include/clang/Basic/CodeGenOptions.def clang/include/clang/Basic/CodeGenOptions.h

[PATCH] D69878: Consoldiate internal denormal flushing controls

cameron.mcinally added inline comments. Comment at: llvm/docs/LangRef.rst:1837 + are present, this overrides ``"denormal-fp-math"``. Not all targets + support separately setting the denormal mode per type. + arsenm wrote: > andrew.w.kaylor wrote: > > cameron.

[PATCH] D69878: Consoldiate internal denormal flushing controls

arsenm marked an inline comment as done. arsenm added inline comments. Comment at: llvm/docs/LangRef.rst:1837 + are present, this overrides ``"denormal-fp-math"``. Not all targets + support separately setting the denormal mode per type. + andrew.w.kaylor wrot

[PATCH] D69878: Consoldiate internal denormal flushing controls

andrew.w.kaylor added inline comments. Comment at: llvm/docs/LangRef.rst:1837 + are present, this overrides ``"denormal-fp-math"``. Not all targets + support separately setting the denormal mode per type. + cameron.mcinally wrote: > andrew.w.kaylor wrote: > >

[PATCH] D69878: Consoldiate internal denormal flushing controls

cameron.mcinally added inline comments. Comment at: llvm/docs/LangRef.rst:1837 + are present, this overrides ``"denormal-fp-math"``. Not all targets + support separately setting the denormal mode per type. + andrew.w.kaylor wrote: > arsenm wrote: > > andrew.w

[PATCH] D69878: Consoldiate internal denormal flushing controls

andrew.w.kaylor added inline comments. Comment at: llvm/docs/LangRef.rst:1837 + are present, this overrides ``"denormal-fp-math"``. Not all targets + support separately setting the denormal mode per type. + arsenm wrote: > andrew.w.kaylor wrote: > > arsenm wr

[PATCH] D69878: Consoldiate internal denormal flushing controls

arsenm marked an inline comment as done. arsenm added inline comments. Comment at: llvm/docs/LangRef.rst:1837 + are present, this overrides ``"denormal-fp-math"``. Not all targets + support separately setting the denormal mode per type. + andrew.w.kaylor wrot

[PATCH] D69878: Consoldiate internal denormal flushing controls

andrew.w.kaylor added inline comments. Comment at: llvm/docs/LangRef.rst:1837 + are present, this overrides ``"denormal-fp-math"``. Not all targets + support separately setting the denormal mode per type. + arsenm wrote: > andrew.w.kaylor wrote: > > Can you d

[PATCH] D69878: Consoldiate internal denormal flushing controls

arsenm marked an inline comment as done. arsenm added inline comments. Comment at: llvm/docs/LangRef.rst:1837 + are present, this overrides ``"denormal-fp-math"``. Not all targets + support separately setting the denormal mode per type. + andrew.w.kaylor wrot

[PATCH] D69878: Consoldiate internal denormal flushing controls

andrew.w.kaylor added inline comments. Comment at: llvm/docs/LangRef.rst:1837 + are present, this overrides ``"denormal-fp-math"``. Not all targets + support separately setting the denormal mode per type. + Can you document which targets do support the option

[PATCH] D69878: Consoldiate internal denormal flushing controls

cameron.mcinally added a comment. Ok, thanks for the clarifications. Looks good to me, but it would be good to have experts in OpenCL/Cuda/AMDGPU review the target specific changes. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69878/new/ https://reviews.llvm.org/D69878

[PATCH] D69878: Consoldiate internal denormal flushing controls

arsenm added a comment. In D69878#1805804 , @arsenm wrote: > In D69878#1801508 , > @cameron.mcinally wrote: > > > This is looking pretty good to me, but I'm ignoring some of the target > > specific code that I'm n

[PATCH] D69878: Consoldiate internal denormal flushing controls

arsenm marked an inline comment as done. arsenm added a comment. In D69878#1801508 , @cameron.mcinally wrote: > This is looking pretty good to me, but I'm ignoring some of the target > specific code that I'm not familiar with. > > Is `denormal-fp-math` i

[PATCH] D69878: Consoldiate internal denormal flushing controls

cameron.mcinally added a comment. This is looking pretty good to me, but I'm ignoring some of the target specific code that I'm not familiar with. Is `denormal-fp-math` influenced by `-Ofast`? Or are there plans for that? Seems like `-Ofast` should imply DAZ and FTZ (if supported by target). I

[PATCH] D69878: Consoldiate internal denormal flushing controls

arsenm added a comment. ping CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69878/new/ https://reviews.llvm.org/D69878 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69878: Consoldiate internal denormal flushing controls

Anastasia added inline comments. Comment at: clang/lib/CodeGen/CGCall.cpp:1775 -if (getLangOpts().OpenCL) - FuncAttrs.addAttribute("denorms-are-zero", - llvm::toStringRef(CodeGenOpts.FlushDenorm)); arsenm wrote: > Anastasia wr

[PATCH] D69878: Consoldiate internal denormal flushing controls

arsenm updated this revision to Diff 233059. arsenm added a comment. Reword langref, fix name in langref CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69878/new/ https://reviews.llvm.org/D69878 Files: clang/include/clang/Basic/CodeGenOptions.def clang/include/clang/Basic/CodeGenOpt

[PATCH] D69878: Consoldiate internal denormal flushing controls

scanon added inline comments. Comment at: llvm/docs/LangRef.rst:1834 +``"denormal-fp-math-f32"`` + Same as ``"denorm-fp-math-f32"``, except for float types. If both + are present, this overrides ``"denorm-fp-math"``. Can you clarify this a little bit? I'd pre

[PATCH] D69878: Consoldiate internal denormal flushing controls

arsenm marked an inline comment as done. arsenm added inline comments. Comment at: clang/lib/CodeGen/CGCall.cpp:1775 -if (getLangOpts().OpenCL) - FuncAttrs.addAttribute("denorms-are-zero", - llvm::toStringRef(CodeGenOpts.FlushDenorm)); ---

[PATCH] D69878: Consoldiate internal denormal flushing controls

Anastasia added inline comments. Comment at: clang/lib/CodeGen/CGCall.cpp:1775 -if (getLangOpts().OpenCL) - FuncAttrs.addAttribute("denorms-are-zero", - llvm::toStringRef(CodeGenOpts.FlushDenorm)); arsenm wrote: > Anastasia wr

[PATCH] D69878: Consoldiate internal denormal flushing controls

arsenm marked an inline comment as done. arsenm added inline comments. Comment at: clang/lib/CodeGen/CGCall.cpp:1775 -if (getLangOpts().OpenCL) - FuncAttrs.addAttribute("denorms-are-zero", - llvm::toStringRef(CodeGenOpts.FlushDenorm)); ---

[PATCH] D69878: Consoldiate internal denormal flushing controls

Anastasia added inline comments. Comment at: clang/lib/CodeGen/CGCall.cpp:1775 -if (getLangOpts().OpenCL) - FuncAttrs.addAttribute("denorms-are-zero", - llvm::toStringRef(CodeGenOpts.FlushDenorm)); so where would `denorms-are-

[PATCH] D69878: Consoldiate internal denormal flushing controls

jlebar added a comment. > AMDGPU wants a distinct control for f32 flushing from f16/f64, and as far as > I can tell the same is true for NVPTX (based on the attribute name). I may be corrected, but I believe nvptx only supports ftz for f32. > Double-precision instructions support subnormal inpu

[PATCH] D69878: Consoldiate internal denormal flushing controls

arsenm updated this revision to Diff 228348. arsenm added a comment. Fix name in documentation CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69878/new/ https://reviews.llvm.org/D69878 Files: clang/include/clang/Basic/CodeGenOptions.def clang/include/clang/Basic/CodeGenOptions.h c

[PATCH] D69878: Consoldiate internal denormal flushing controls

arsenm marked an inline comment as done. arsenm added a comment. In D69878#1736865 , @Anastasia wrote: > > Stop emitting the denorms-are-zero attribute for the OpenCL flag. It > > has no in-tree users. The meaning would also be target dependent, such > >

[PATCH] D69878: Consoldiate internal denormal flushing controls

Anastasia added a comment. > Stop emitting the denorms-are-zero attribute for the OpenCL flag. It > has no in-tree users. The meaning would also be target dependent, such > as the AMDGPU choice to treat this as only meaning allow flushing of > f32 and not f16 or f64. The naming is also potentia

[PATCH] D69878: Consoldiate internal denormal flushing controls

arsenm updated this revision to Diff 228167. arsenm added a comment. Rename subnormal to denormal. Will defer splitting input and output setting into a future patch before switching default behavior CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69878/new/ https://reviews.llvm.org/D6987

[PATCH] D69878: Consoldiate internal denormal flushing controls

arsenm marked an inline comment as done. arsenm added inline comments. Comment at: llvm/docs/LangRef.rst:1828-1831 + be flushed to zero by standard floating point operations. It is not + mandated that flushing to zero occurs, but if a subnormal output is + flushed to zero,

[PATCH] D69878: Consoldiate internal denormal flushing controls

arsenm marked an inline comment as done. arsenm added inline comments. Comment at: llvm/docs/LangRef.rst:1828-1831 + be flushed to zero by standard floating point operations. It is not + mandated that flushing to zero occurs, but if a subnormal output is + flushed to zero,

[PATCH] D69878: Consoldiate internal denormal flushing controls

arsenm created this revision. arsenm added reviewers: scanon, spatel, cameron.mcinally, andrew.w.kaylor, tra, jlebar, Anastasia, yaxunl. Herald added subscribers: hiraditya, kristof.beyls, tpr, nhaehnle, wdng, jvesely, jholewinski. Herald added a project: LLVM. arsenm added parent revisions: D695