[PATCH] D69598: Work on cleaning up denormal mode handling

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: clang/lib/CodeGen/CGCall.cpp:1736-1737
+    if (CodeGenOpts.FPSubnormalMode != llvm::SubnormalMode::Invalid)
+      FuncAttrs.addAttribute("denormal-fp-math",
+                             llvm::subnormalModeName(CodeGenOpts.FPSubnormalMode));
 
----------------
arsenm wrote:
> spatel wrote:
> > Do you plan to change the attribute string from "denormal" to "subnormal" as part of upgrading it to work per-FP-type? Would we need to auto-upgrade old IR as part of making the string consistent with the code?
> > 
> > Can we stash the attribute string name inside a getter function in the new ADT file, so clang and LLVM have a common source of truth for the attribute name?
> I'm considering it, but at the moment I'm trying to avoid changes. The next step I'm working on is adding denormal-fp-math-f32 (or maybe subnormal-fp-math-f32), which will co-exist and override the current attribute if the type matches
I think it would be better to not change the vocabulary incrementally then. Ie, keep everything "denormal" in this patch, and then universally change the terminology to "subnormal" in one step. That way we won't have any inconsistency/confusion between the attribute name and the code.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69598/new/

https://reviews.llvm.org/D69598