[PATCH] D72350: [DDG] Data Dependence Graph - Graph Simplification

[Bardia Mahjour via Phabricator via llvm-commits]

bmahjour added inline comments.


================
Comment at: llvm/lib/Analysis/DependenceGraphBuilder.cpp:431
+    NodeType &Src = **CandidateSourceNodes.begin();
+    CandidateSourceNodes.erase(&Src);
+    assert(Src.getEdges().size() == 1 &&
----------------
Meinersbur wrote:
> [serious] Since `DenseMap::erase` replaces the item with a tombstone, searching for the next non-tombstone (`DenseMap::begin`) will be linear in time. That is, this while-loop takes O(n^2) just for iterating over the elements.
> 
> Maybe have a separate worklist+hastable for marking elements already processed?
I was trying to avoid a second container to avoid memory waste, but since the worklist is transient and typically small, duplicating it should be fine. I've also found a bug where (depending on the order) merging nodes can trigger the "Expected a single edge from the candidate src node." assert. I've fixed this as well and added comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72350