[PATCH] D58170: [DTU] Refine the logic of deduplication

[Brian Rzycki via Phabricator via llvm-commits]

brzycki added inline comments.


================
Comment at: llvm/lib/Analysis/DomTreeUpdater.cpp:284
+        Seen.insert(Edge);
+        // If the update doesn't appear in the CFG, it means that
+        // either the change isn't made or relevant operations
----------------
@NutshellySima it's probably me but I don't see how the comments match the code here. Let's take this example:
```
Strategy: Lazy
CFG: edge exists between A->B
Updates: {{Delete, A, B}, {Insert, A, B}}
U: {Delete, A, B}
Edge: {A,B}
isUpdateValid: true

```
In this case we will enqueue a deletion of edge A->B and not process the second update, the `{Insert, A, B}` because that `Edge` already exists in `Seen`.  If a `flush()` is called immediately after the state of `DTU` is incorrect.

Am I missing something here that prevents this case?


================
Comment at: llvm/unittests/Analysis/DomTreeUpdaterTest.cpp:793
+}
\ No newline at end of file

----------------
Nit: please add a newline.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58170