[PATCH] D24031: [InstCombine] replace fold of an icmp pattern that should never happen with an assert

[Sanjay Patel via llvm-commits]

spatel added a comment.

In https://reviews.llvm.org/D24031#530611, @efriedma wrote:

> Are you absolutely sure we can't trigger the assertion?  I don't think there's any guarantee instcombine will visit the "and" before the "icmp".


Hmm...that would kill my (possibly wrong) mental model of InstCombine as a top-down / greedy machine. Is the comment in AddReachableCodeToWorklist() enough?

  // Once we've found all of the instructions to add to instcombine's worklist,
  // add them in reverse order.  This way instcombine will visit from the top
  // of the function down.

If that's not a valid assumption or if there's some other way that the worklist order doesn't match the instruction order, then I agree we can't assert here.

In that case, I think it should still be valid to remove the block of code (but don't add the assert). The regression test verifies that we get the end result that we want.


https://reviews.llvm.org/D24031