[PATCH] D63147: [MemorySSA] Add additional verification for phis.

[George Burgess IV via Phabricator via llvm-commits]

george.burgess.iv added a comment.

Thanks for adding this!

just a code clarity nit. LGTM otherwise



================
Comment at: lib/Analysis/MemorySSA.cpp:1873
+        auto *IncAcc = Phi->getIncomingValue(I);
+        while (true) {
+          if (auto *DefList = getBlockDefs(Pred)) {
----------------
nit: IMO, the control-flow of this loop is a bit difficult to follow, and the verification it's doing about forwards-unreachable code are pretty understated. do you think something like the following is clearer (and functionally equivalent)?

```
if (DT->getNode(Pred))) {
  auto *CurBB = Pred;
  while (CurBB) {
    if (/* getblockdefs(CurBB) */) {
      assert(LastAcc == IncAcc);
      break;
    }
    CurBB = IDom;
  }
  assert(CurBB || IncAcc == LOE && ...);
} else if (/* getblockdefs(Pred) */) {
  assert(IncAcc == LastAcc);
} else {
  assert(IncAcc == LOE);
}
```


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63147