[PATCH] D123835: AMDGPU/SDAG: Refine the fold to v_mad_[iu]64_[iu]32

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:10696
 
-  SDValue MulLHS = LHS.getOperand(0);
-  SDValue MulRHS = LHS.getOperand(1);
-  SDValue AddRHS = RHS;
+  // Avoid the fold if it would unduly increase the number of multiply due to
+  // multiple uses, except on hardware with full-rate multiply-add (which is
----------------
s/number of multiply/number of multiplies/


================
Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:10702
+    for (auto I = LHS->use_begin(), E = LHS->use_end(); I != E; ++I) {
+      if (I.getUse().getResNo() != 0)
+        continue;
----------------
I don't understand why you're checking this if you bail on not ISD:ADD. I guess it would make sense if you were handling the carry out adds in a separate patch?


================
Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:10747
+
+  SDValue Shift = DAG.getShiftAmountConstant(32, MVT::i64, SL);
+  SDValue AccumLo = DAG.getNode(ISD::TRUNCATE, SL, MVT::i32, Accum);
----------------
Could just hardcode this to i32 instead of going through getShiftAmountConstant


================
Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:10752-10757
+  if (!MulLHSUnsigned32) {
+    SDValue MulLHSHi = DAG.getNode(ISD::SRL, SL, MVT::i64, MulLHS, Shift);
+    MulLHSHi = DAG.getNode(ISD::TRUNCATE, SL, MVT::i32, MulLHSHi);
+    SDValue MulHi = DAG.getNode(ISD::MUL, SL, MVT::i32, MulLHSHi, MulRHSLo);
+    AccumHi = DAG.getNode(ISD::ADD, SL, MVT::i32, MulHi, AccumHi);
+  }
----------------
A comment with the DAG formed here would be helpful 


================
Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:10766
+
+  AccumHi = DAG.getNode(ISD::ANY_EXTEND, SL, MVT::i64, AccumHi);
+  AccumHi = DAG.getNode(ISD::SHL, SL, MVT::i64, AccumHi, Shift);
----------------
Ditto


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123835