[PATCH] D81558: [NewPM] Introduce PreserveCFG check

[Jakub Kuderski via Phabricator via llvm-commits]

kuhar requested changes to this revision.
kuhar added a comment.
This revision now requires changes to proceed.

I think this deserves some extra documentation that explains what it means to preserve CFG in plain English. For example, should we also care about preserving the number of return blocks?



================
Comment at: llvm/include/llvm/Passes/StandardInstrumentations.h:77
+  // that the set is unordered because order of succesors may be changed.
+  using CFG = DenseMap<const BasicBlock *, SmallSet<const BasicBlock *, 4>>;
+
----------------
CFG is a multigraph. A basic block can have duplicate successors, i.e., there may be many outgoing edges to the same basic block.
Shouldn't this be a map<BB, set<pair<BB, count>> or map<BB, multiset<BB, count>>?


================
Comment at: llvm/include/llvm/Passes/StandardInstrumentations.h:77
+  // that the set is unordered because order of succesors may be changed.
+  using CFG = DenseMap<const BasicBlock *, SmallSet<const BasicBlock *, 4>>;
+
----------------
kuhar wrote:
> CFG is a multigraph. A basic block can have duplicate successors, i.e., there may be many outgoing edges to the same basic block.
> Shouldn't this be a map<BB, set<pair<BB, count>> or map<BB, multiset<BB, count>>?
Is tracking BasicBlock* enough? What if blocks get deleted and the tracked pointers become invalid? Does it make sense to use WeakVH instead?


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

https://reviews.llvm.org/D81558