[PATCH] D136201: [InstCombine] Handle PHI nodes in PtrReplacer

[Nikita Popov via Phabricator via llvm-commits]

nikic added a comment.

Logic looks correct to me now. Looks like the test changes got lost though?



================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp:15
 #include "llvm/ADT/MapVector.h"
+#include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/SmallString.h"
----------------
Shouldn't be needed (already used in file).


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp:264
 
-  bool collectUsers(Instruction &I);
+  bool collectUsers(Instruction &I, MemTransferInst *Copy);
   void replacePointer(Instruction &I, Value *V);
----------------
Copy argument no longer needed


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp:287
+  for (auto *V : ValuesToRevisit)
+    if (!Worklist.contains(cast<Instruction>(V)))
+      return false;
----------------
Could make ValuesToRevisit use `Instruction *` rather than `Value *` and save this cast.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp:315
+      }
+      if (ValueInserted)
+        continue;
----------------
Can use `any_of` instead of `for` here and avoid the ValueInserted variable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136201