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:
> > > 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
> I'm not seeing the value in using "subnormal" relative to the confusion cost
> then. If we're always going to use the "denormal" flag in clang, then I'd
> prefer to have the code be consistent with that name. That's what I'd grep
> for, so I think that's what anyone viewing the code for the first time would
> expect too.
I thought it might be good to move towards the current standard terminology,
but it's not critical
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
