[PATCH] D55957: [MemorySSA] Create walker that skips starting access.

[George Burgess IV via Phabricator via llvm-commits]

george.burgess.iv added a comment.

Thanks for this!



================
Comment at: lib/Analysis/MemorySSA.cpp:976
 
+class MemorySSA::SkipSelfWalker final : public MemorySSA::CachingWalker {
+ public:
----------------
ClobberWalker is pretty big (nearly 2K). Can we instead make SkipSelfWalker carry around a pointer to `MSSA->getWalker()` (or just MSSA. I'm not picky)?

Since CachingWalker is opaque (modulo overrides from MemorySSAWalker) to everything that isn't in this file, I have no issue with adding "don't touch these unless ..." methods to its public API.


================
Comment at: lib/Analysis/MemorySSA.cpp:977
+class MemorySSA::SkipSelfWalker final : public MemorySSA::CachingWalker {
+ public:
+  SkipSelfWalker(MemorySSA *MSSA, AliasAnalysis *AA, DominatorTree * DT)
----------------
please clang-format this 


================
Comment at: lib/Analysis/MemorySSA.cpp:979
+  SkipSelfWalker(MemorySSA *MSSA, AliasAnalysis *AA, DominatorTree * DT)
+      : CachingWalker(MSSA,AA, DT) {};
+
----------------
nit: no trailing semi, please


================
Comment at: lib/Analysis/MemorySSA.cpp:1462
+  return TransformWalker.get();
+};
+
----------------
No trailing `;`, please


================
Comment at: lib/Analysis/MemorySSA.cpp:2275
+MemoryAccess *
+MemorySSA::SkipSelfWalker::getClobberingMemoryAccess(MemoryAccess *MA) {
+  LLVM_DEBUG(dbgs() << "Starting Memory SSA clobber for: " << *MA << " is ");
----------------
Please do the refactoring/dedup as a part of this patch, rather than as a follow up


Repository:
  rL LLVM

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

https://reviews.llvm.org/D55957