[PATCH] D98050: [AMDGPU][GlobalISel] Transform (fsub (fpext (fneg (fmul x, y))), z) -> (fneg (fma (fpext x), (fpext y), z))

[Matt Arsenault via Phabricator via llvm-commits]

arsenm requested changes to this revision.
arsenm added inline comments.
This revision now requires changes to proceed.


================
Comment at: llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp:4481-4482
+
+  bool LegalOperations =
+      isLegal({TargetOpcode::G_FADD, {DstType, SrcType}});
+  // Floating-point multiply-add with intermediate rounding.
----------------
First, fadd only has as single type index. Second, I don't think there's much point in checking it's legality. It doesn't imply "LegalOperations"


================
Comment at: llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp:4488
+      TLI.isFMAFasterThanFMulAndFAdd(*MF, DstType) &&
+      (!LegalOperations || isLegal({TargetOpcode::G_FMA, {DstType, SrcType}}));
+
----------------
We have a separate check for before the legalizer. FMA also only has one type index


================
Comment at: llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp:4527
+  //      (fneg (fma (fpext x), (fpext y), z))
+  if (MI0->getOpcode() == TargetOpcode::G_FNEG) {
+    MachineInstr *MI00 = MRI.getVRegDef(MI0->getOperand(1).getReg());
----------------
Should probably be using mi_matches for all of these instead of your own getVRegDefs and nested ifs


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

https://reviews.llvm.org/D98050