[PATCH] D138899: [DAGCombiner] handle more store value forwarding

[Nemanja Ivanovic via Phabricator via llvm-commits]

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

LGTM. The minor nits do not require another review.

I'm not sure if @tstellar has anything further to add or if he objects to the AMDGPU changes.



================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:365
 
+    StoreSDNode *getUniqueStoreFeeding(LoadSDNode *LD, int64_t &Offset);
     // Scalars have size 0 to distinguish from singleton vectors.
----------------
I think a comment describing this function would be useful. Something like:
```
// Looks up the chain to find a unique (unaliased) store feeding the
// passed load. If no such store is found, returns a nullptr.
// Note: This will look past a CALLSEQ_START if the load is
//       chained to it so that it can find stack stores for byval
//       params.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:17330
+      BaseIndexOffset BasePtrST = BaseIndexOffset::match(Store, DAG);
+      if (BasePtrST.equalBaseIndex(BasePtrLD, DAG, Offset)) {
+        // Make sure the store is not aliased with any nodes in TokenFactor.
----------------
Nit: you can probably reduce nesting a bit with
```
if (!BasePtrST.equalBaseIndex(BasePtrLD, DAG, Offset))
  continue;
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138899