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

Wed Oct 7 09:46:15 PDT 2020

foad marked 4 inline comments as done.
foad added inline comments.

================
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.
----------------
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.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88955