[PATCH] D81558: [NewPM] Introduce PreserveCFG check
Yevgeny Rouban via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Aug 4 22:37:06 PDT 2020
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
More information about the llvm-commits
mailing list