[PATCH] D98047: [AMDGPU][GlobalISel] Transform (fadd (fma x, y, (fpext (fmul u, v))), z) -> (fma x, y, (fma (fpext u), (fpext v), z))

[Mirko Brkusanin via Phabricator via llvm-commits]

mbrkusanin added inline comments.


================
Comment at: llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp:4520
+      Register FpExtU =
+          Builder.buildFPExt(DstType, FMulMI->getOperand(1).getReg())
+                 .getReg(0);
----------------
Use B not Builder. Same for everywhere else below.


================
Comment at: llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp:4533-4537
+      LLT Ty1 = MRI.getType(X), Ty2 = MRI.getType(Y);
+      if (Ty1 != DstType && Ty2 != DstType) {
+        X = Builder.buildFPExt(DstType, X).getReg(0);
+        Y = Builder.buildFPExt(DstType, Y).getReg(0);
+      }
----------------
These are fpext for X and Y which do not happen in this pattern. Just remove these lines.


================
Comment at: llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp:4570-4574
+        LLT Ty1 = MRI.getType(X), Ty2 = MRI.getType(Y);
+        if (Ty1 != DstType && Ty2 != DstType) {
+          X = Builder.buildFPExt(DstType, X).getReg(0);
+          Y = Builder.buildFPExt(DstType, Y).getReg(0);
+        }
----------------
You already checked above for m_GFPExt(m_MInstr(FMAMI))
In this case you always need to build fpext for X and Y.


================
Comment at: llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp:4606-4609
+      if (Ty1 != DstType && Ty2 != DstType) {
+        X = Builder.buildFPExt(DstType, X).getReg(0);
+        Y = Builder.buildFPExt(DstType, Y).getReg(0);
+      }
----------------
(same as first fold)
These are fpext for X and Y which do not happen in this pattern. Just remove these lines.


================
Comment at: llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp:4643-4646
+        if (Ty1 != DstType && Ty2 != DstType) {
+          X = Builder.buildFPExt(DstType, X).getReg(0);
+          Y = Builder.buildFPExt(DstType, Y).getReg(0);
+        }
----------------
(same as second fold)
You already checked above for m_GFPExt(m_MInstr(FMAMI))
In this case you always need to build fpext for X and Y.


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

https://reviews.llvm.org/D98047