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

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUInstCombineIntrinsic.cpp:166
 
+static bool isConstantFPZero(Value *V) {
+  if (auto *C = dyn_cast<ConstantFP>(V))
----------------
arsenm wrote:
> Do we not have something for this in PatternMatch?
Does the sign matter?


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUInstCombineIntrinsic.cpp:166-176
+static bool isConstantFPZero(Value *V) {
+  if (auto *C = dyn_cast<ConstantFP>(V))
+    return C->isZero();
+  return false;
+}
+
+static bool isConstantFPFiniteNonZero(Value *V) {
----------------
Do we not have something for this in PatternMatch?


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUInstCombineIntrinsic.cpp:842
+
+    // The legacy behaviour is that multiplying zero by anything, even NaN or
+    // infinity, gives +0.0.
----------------
Clarify if the sign of zero matters?


================
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.
----------------
Should check this case first since it's cheaper


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88955