[PATCH] D86589: (Expensive) Check for Loop, SCC and Region pass return status

[Mikael Holmén via Phabricator via llvm-commits]

uabelho added inline comments.


================
Comment at: llvm/lib/IR/StructuralHash.cpp:51
+
+      const Instruction *Term = BB->getTerminator();
+      for (unsigned i = 0, e = Term->getNumSuccessors(); i != e; ++i) {
----------------
fhahn wrote:
> uabelho wrote:
> > Hi!
> > What if the BB doesn't have a terminator at all? Then Term will be nullptr which I don't think is good..
> > 
> > (This happens for us in the unit test PassManager.CallGraphUpdater0 with expensive checks when we run in our downstream clone)
> I'd assume this should only be called for valid IR, which means there must be terminator? 
> 
> Unrelated, but could this just be something like `for (auto &Succ : successors(BB)) {`?
I see. Thanks!

(The testcase was originally not behaving well (it inserted a non-terminator after the terminator in a bb), but that was fixed in  3667d87. Unfortunately we have lost that fix downstream during some EXPENSIVE_CHECKS fiddling we've done and then ran into this problem.)

No need to change the successor iteration then.

Thanks again!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86589