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

[Matt Arsenault via Phabricator via llvm-commits]

arsenm marked an inline comment as done.
arsenm added a comment.

In D69547#1728689 <https://reviews.llvm.org/D69547#1728689>, @cameron.mcinally wrote:

> Is `isFMAFasterThanFMulAndFAdd`'s `MachineFunction` argument currently used? Or is that coming in a follow-up Diff? I might have missed it...


D69583 <https://reviews.llvm.org/D69583> starts using it (although it's still effectively a no-op until it ultimately uses a function attribute instead of a subtarget feature)



================
Comment at: llvm/include/llvm/CodeGen/TargetLowering.h:2549
     return false;
   }
 
----------------
cameron.mcinally wrote:
> Agreed, the AArch64 requirements are ugly.
> 
> It doesn't look like the AArch64 hooks need the Function arg. Could we just make the MachineFunction argument optional; to consolidate the two functions? E.g.
> 
> ```
>   virtual bool isFMAFasterThanFMulAndFAdd(EVT, const MachineFunction &MF = nullptr) const {
> ```
> 
> Or is that not allowed with some virtual functions? I can't remember the details...
> 
That would still be using EVT, which I would prefer to avoid spreading. I also wouldn't like using an optional argument here, because then there's a risk of new uses not passing it


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

https://reviews.llvm.org/D69547