[PATCH] D88955: [AMDGPU] Add simplification/combines for llvm.amdgcn.fmul.legacy

[Matt Arsenault via Phabricator via llvm-commits]

arsenm accepted this revision.
arsenm added inline comments.
This revision is now accepted and ready to land.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUInstCombineIntrinsic.cpp:856-857
+      CanSimplifyToMul = true;
+    } else if (isConstantFPFiniteNonZero(Op0) ||
+               isConstantFPFiniteNonZero(Op1)) {
+      // One operand is not zero or infinity or NaN.
----------------
foad wrote:
> arsenm wrote:
> > arsenm wrote:
> > > Should check this case first since it's cheaper
> > Actually, should we have a more general isKnownFiniteNonZero that happens to check constants?
> Perhaps. ValueTracking has `isKnownNonZero` which doesn't work on floats, but could be made to. It also has `CannotBeOrderedLessThanZero`.
> 
> `isKnownFiniteNonZero` seems rather specific to me. I realise it's probably more efficient than separate calls to `isKnownNeverNaN`, `isKnownNeverInfinity` and `isKnownNonZero`, but without a lot of refactoring I think there's a serious risk of having dozens of different entrypoints.
> 
> As another data point, SelectionDAG has separate `isKnownNeverZero` and `isKnownNeverZeroFloat` functions.
> 
> Anyway... I decided to duck all of this and just have it work on constants, for now.
> 
Should add todo to generalize. I was also thinking all the isKnown* functions should return a bitmask, kind of like the inverse of the class intrinsic


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88955