jyknight added inline comments.

================
Comment at: llvm/lib/CodeGen/MachineVerifier.cpp:701
+      } else if (MBB->succ_size() == LandingPadSuccs.size() ||
+                 MBB->succ_size() == IndirectPadSuccs.size()) {
         // It's possible that the block legitimately ends with a noreturn
----------------
This isn't correct.

This line here, is looking at a block which doesn't end in a jump to a 
successor. So, it's trying to verify that the successor list makes sense in 
that context.

The unstated assumption in the code is that the only successors will be landing 
pads. Instead of actually checking each one, instead it just checks that the 
count is the number of landing pads, with the assumption that all the 
successors should be landing pads, and that all the landing pads should be 
successors.

The next clause is then checking for the case where there's a fallthrough to 
the next block. In that case, the successors should've been all the landing 
pads, and the single fallthrough block.

Adding similar code to check for the number of callbr targets doesn't really 
make sense. It's certainly not the case that all callbr targets are targets of 
all 
callbr instructions. And even if it was, this still wouldn't be counting things 
correctly.


However -- I think i'd expect analyzeBranch to error out (returning true) when 
confronted by a callbr instruction, because it cannot actually tell what's 
going on there. If that were the case, nothing in this block should even be 
invoked. But I guess that's probably not happening, due to the terminator being 
followed by non-terminators.

That seems likely to be a problem that needs to be fixed. (And if that is 
fixed, I think the changes here aren't needed anymore)




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69868



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to