[PATCH] D47441: SafepointIRVerifier should ignore dead blocks and dead edges

[Yevgeny Rouban via Phabricator via llvm-commits]

yrouban marked 4 inline comments as done.
yrouban added inline comments.


================
Comment at: lib/IR/CFGDeadness.cpp:127
+      const Use &U = PU.getUser()->getOperandUse(PU.getOperandNo());
+      assert(!result &&
+             "Phi is not expected to refer to one block several times");
----------------
anna wrote:
> I think the assert and the corresponding ifdef compilation with break isn't very useful. It's just testing a property of the IRVerifier for phi nodes. 
> Also, conditional compilation with behaviour change is not very preferable. Could we get rid of this?
In debug mode these two assertions (1) and (2) check that for this phi node and the specified basic block there is at leats one edge (2) and no more than one edge (1). To check (1) we need to iterate through the whole set of predecessors, that is why we should not break at the first found edge. Any other way to express this more elegantly?
This assertion seems obvious, but I did not find a place in LLVM (other than the LL parser) that prevents phis with duplicate pairs like phi [%value1, %block1], [%value1, %block1]. In this case, how many incoming edges do we have, 1 or 2?


================
Comment at: lib/IR/CFGDeadness.cpp:160
+INITIALIZE_PASS_BEGIN(CFGDeadnessWrapperPass, "cfgdeadness",
+                      "CFG Deadness", true, true)
+INITIALIZE_PASS_DEPENDENCY(DominatorTreeWrapperPass)
----------------
anna wrote:
> it doesn't modify CFG, so the cfg arg should be false.  
Hm, it is not obvious from the sources.
But why DominatorTreeWrapperPass has its set to true then?
  INITIALIZE_PASS(DominatorTreeWrapperPass, "domtree", "Dominator Tree Construction", true, true)



https://reviews.llvm.org/D47441