arsenm marked an inline comment as done.
arsenm 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));
----------------
spatel wrote:
> 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.
In the follow up patch, the new attribute uses the old denormal name. The
clang option handling maintains the old name to match the flag, but the new
internal enums and functions use the subnormal name. Is that a reasonable
state? I don’t want to spread the old name through the new utilities, but also
don’t want to have to auto upgrade bitcode for a name change
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D69598/new/
https://reviews.llvm.org/D69598
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
