[PATCH] D122105: [SystemZ] Patchset for expanding memcpy/memset using at most 2 stores.

[Ulrich Weigand via Phabricator via llvm-commits]

uweigand added a comment.

Looks good to me in principle, just a couple suggestions for refactoring inline.   Before committing this, we should confirm the performance impact.



================
Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:201
+  if (VT == MVT::Untyped)
+    return false;
 
----------------
I'm wondering if it would be better to override findOptimalMemOpLowering and handle that check there.


================
Comment at: llvm/lib/Target/SystemZ/SystemZISelLowering.cpp:818
+  findSplat();
+}
+
----------------
Maybe it would be simpler to have the `APFloat` constructor only set `isFP128` and then call into the `APInt` constructor with `FPImm.bitcastToAPInt()`.   The we also wouldn't need a separate `findSplat` routine.


================
Comment at: llvm/lib/Target/SystemZ/SystemZISelLowering.cpp:1015
+    return MVT::Untyped;  // Memset zero: Use XC
+
+  return Subtarget.hasVector() ? MVT::v2i64 : MVT::Other;
----------------
So all the above checks would be in an overloaded findOptimalMemOpLowering, and the overloaded getOptimalMemOpType would consist solely of the line below.


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

https://reviews.llvm.org/D122105