[PATCH] D81558: [NewPM] Introduce PreserveCFG check

[Jakub Kuderski via Phabricator via llvm-commits]

kuhar 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>>;
+
----------------
yrouban wrote:
> 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.
AFAIK in C++, when pointee's lifetime ends, the pointer becomes invalid and you cannot use it in any meaningful way, even when you don't dereference it. It'd be incorrect to print it, compare with another pointers, etc. This would also make debugging very confusing.


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

https://reviews.llvm.org/D81558