[PATCH] D32086: [DAG] Improve store merge candidate pruning.

Sanjay Patel via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Apr 18 08:38:39 PDT 2017


spatel accepted this revision.
spatel added a comment.
This revision is now accepted and ready to land.

LGTM, but see inline for nits in code comments.



================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:12380
+  // works out as if successful, as the remaining stores are checked
+  // after the first collection of stores is merged. However in the
+  // case that a non-mergeable store is found first (e.g., p[-2],
----------------
comma after "However"


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:12382
+  // case that a non-mergeable store is found first (e.g., p[-2],
+  // p[0],p[1],p[2],p[3]} we would fail and miss the subsequent
+  // mergable cases. To prevent this, we prune such stores from the
----------------
mismatched (}
comma before "we" 


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:12383
+  // p[0],p[1],p[2],p[3]} we would fail and miss the subsequent
+  // mergable cases. To prevent this, we prune such stores from the
+  // front of StoreNodes here.
----------------
typo: mergeable


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:12392
+
+  // Bail if we've don't have enough Candidates to do a merge.
+  if (StartIdx + 1 >= StoreNodes.size())
----------------
we've -> we
Candidates -> candidates
to do a -> to


https://reviews.llvm.org/D32086





More information about the llvm-commits mailing list