[PATCH] D53552: [DAGCombine] Improve alias analysis for chain of independent stores.

[Clement Courbet via Phabricator via llvm-commits]

courbet added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:19008
+  // Intervals records which offsets from BaseIndex have been covered. In
+  // the common case, ever store writes to the immediately previous address
+  // space and thus merged with the previous interval at insertion time.
----------------
every*


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:19022
   if (!BasePtr.getBase().getNode())
-    return false;
+    return St;
 
----------------
Why ? This is `return true` AFAICT...


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:19026
   if (BasePtr.getBase().isUndef())
-    return false;
+    return St;
 
----------------
ditto


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:19049
+    auto I = Intervals.find(Offset);
+    // If there's a next intervals, we should end before it.
+    if (I != Intervals.end() && I.start() < (Offset + Length))
----------------
interval*


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:19126
+
+  // Holds all stores chained to St
+  if (parallelizeChainedStores(St))
----------------
Please fix the comment.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:19127
+  // Holds all stores chained to St
+  if (parallelizeChainedStores(St))
+    return true;
----------------
this returns true if St has no base and offset. This is weird. I know that this cannot happen because we just made the check above, but it's misleading and redundant.  Maybe pass `BasePtr` to `findBetterNeighborChains()` and assert ?


Repository:
  rL LLVM

https://reviews.llvm.org/D53552