[PATCH] D61843: [DAGCombine] Match a pattern where a wide type scalar value is stored by several narrow stores

[Artur Pilipenko via Phabricator via llvm-commits]

apilipenko added a comment.

Have you considered reusing calculateByteProvider mechanism from load combining instead of matching an exact pattern?



================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:6219-6233
+    // All the stores store different byte of the CombinedValue. A truncate is
+    // required to get that byte value.
+    SDValue Trunc = Store->getValue();
+    if (Trunc.getOpcode() != ISD::TRUNCATE)
+      return SDValue();
+    // A shift operation is required to get the right byte offset, except the
+    // first byte.
----------------
Can you add a comment outlining the pattern you are looking for? It would be easier to grasp from a short comment than from the implementation.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:6251
+        CombinedValue = Value;
+      // Give up if the combined value type is small than the store size.
+      if (CombinedValue.getValueSizeInBits() < VT.getSizeInBits())
----------------
Typo "small" -> "smaller"


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:6318
+
+  DAG.ReplaceAllUsesWith(N, NewStore.getNode());
+  return NewStore;
----------------
What happens to the individual stores after? Do you rely on other DAG combine rules to remove them?


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

https://reviews.llvm.org/D61843