[PATCH] D81558: [NewPM] Introduce PreserveCFG check

[Yevgeny Rouban via Phabricator via llvm-commits]

yrouban added inline comments.


================
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:
> 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?
Ok. I had in mind checking PostDomTree invariant that must be insensitive to parallel edges.
I will try to make it stricter as you suggest.


================
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>>;
+
----------------
yrouban wrote:
> kuhar wrote:
> > 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?
> Ok. I had in mind checking PostDomTree invariant that must be insensitive to parallel edges.
> I will try to make it stricter as you suggest.
To not complicate things with WeakVH I decided to not dereference the blocks collected before the checked pass unless I see the same pointer in the CFG after the pass. See the //printCFGdiff()//.
WeakVH would allow to check accurately, but could not be used as a set/map key.


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

https://reviews.llvm.org/D81558