[PATCH] D59569: [MemorySSA] Limit clobber walks.

[George Burgess IV via Phabricator via llvm-commits]

george.burgess.iv added a comment.

Thanks for this!

Would it be straightforward to have a lit test (or similar) that sets `memssa-check-limit` to something super low and checks that this all does what we want (e.g. walk limit is cumulative across all walks on OptimizePhis, the walker trivially obeys it, ...)



================
Comment at: lib/Analysis/MemorySSA.cpp:557
+
+        if (--UpwardWalkLimit == 0)
+          return {Current, true, MayAlias};
----------------
nit: please remove `== 0`


================
Comment at: lib/Analysis/MemorySSA.cpp:910
+  MemoryAccess *findClobber(MemoryAccess *Start, UpwardsMemoryQuery &Q,
+                            unsigned UpwardWalkLimit) {
     Query = &Q;
----------------
Was this intended to be `unsigned &UpwardWalkLimit`? Looks like we're passing it by ref everywhere else


================
Comment at: lib/Analysis/MemorySSA.cpp:923
+    UpwardsWalkResult WalkResult =
+        walkToPhiOrClobber(FirstDesc, UpwardWalkLimit);
     MemoryAccess *Result;
----------------
Would it be better to make `UpwardWalkLimit` a member? This entire class is a container of query-specific state anyway.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D59569