[PATCH] D69547: DAG: Add function context to isFMAFasterThanFMulAndFAdd

Sanjay Patel via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Nov 18 09:29:33 PST 2019


spatel added reviewers: craig.topper, lebedev.ri, RKSimon.
spatel accepted this revision.
spatel added a comment.
This revision is now accepted and ready to land.

In D69547#1750122 <https://reviews.llvm.org/D69547#1750122>, @arsenm wrote:

> In D69547#1750023 <https://reviews.llvm.org/D69547#1750023>, @spatel wrote:
>
> > I might be missing some part of the bigger picture, but shouldn't we nail down the format of the denorm function attribute before this? It seems unnecessarily wide/vague to pass the entire MachineFunction as the param if all we care about is the 1 denorm attribute setting.
>
>
> The fact that AMDGPU cares about the denormal mode context in this case is a target specific detail. It would be stranger to be passing specifically that in for every user. Another target might have more exotic constraints. AMDGPU does have other specific FP control bits that don't specifically matter here, but another target could have others that do.


Fair enough. I'd choose the other way until there was some proven need to code it this way, but that's just a matter of taste.
It's NFC at this point either way, so LGTM. 
Adding a few more potential SDAG reviewers, so wait a day or so to see if anyone else has comments.


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

https://reviews.llvm.org/D69547





More information about the llvm-commits mailing list