[PATCH] D109953: [X86][FP16] Combine the FADD(A, FMA(B, C, 0)) to FMA(B, C, A)

Pengfei Wang via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Sep 22 02:28:57 PDT 2021


pengfei requested changes to this revision.
pengfei added a comment.
This revision now requires changes to proceed.

The format in this file was wrongly formatted.



================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:47533
+auto AllowContract = [&DAG](const SDNodeFlags &Flags) {
+  return DAG.getTarget().Options.AllowFPOpFusion == FPOpFusion::Fast ||
+         (Flags.hasNoNaNs() && Flags.hasNoInfs() && Flags.hasNoSignedZeros() &&
----------------
LiuChen3 wrote:
> Maybe we can just check hasNoSignedZeros() and hasAllowContract() as pengfei said?
Yeah, I prefer to checking both in line 47582.


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:47582
+    if ((Opcode == X86ISD::VFMADDC || Opcode == X86ISD::VFCMADDC) &&
+        ((ISD::isBuildVectorAllZeros(Op0->getOperand(0).getNode()) &&
+          AllowContract(Op0->getFlags())) ||
----------------
Should this be
```
AllowContract(Op0->getFlags()) && (ISD::isBuildVectorAllZeros(Op0->getOperand(0).getNode()) && Op0->getFlags().hasNoSignedZeros()) || IsVectorAllNegativeZero(Op0->getOperand(0).getNode()))
```
I.e, check `AllowContract` together with `IsVectorAllNegativeZero` as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109953



More information about the llvm-commits mailing list