[PATCH] D61089: [Reassociation] Place moved instructions after landing pads

[David Greene via Phabricator via llvm-commits]

greened marked an inline comment as done.
greened added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/Reassociate.cpp:874
+      // the landingpad.
+      while (isa<PHINode>(InsertPt) || isa<LandingPadInst>(InsertPt))
+        ++InsertPt;
----------------
efriedma wrote:
> greened wrote:
> > efriedma wrote:
> > > Explicitly checking for LandingPad isn't sufficient on Windows.  There are two issues: one, you need to use isEHPad() to check for all the relevant exception-handling instructions, and two, you can't insert a negate into a block with a catchswitch.
> > This code moves the negate around within the local block, so presumably the negate is already in the block and therefore there should be no `catchswitch`, right?
> > 
> > I will update this to use `isEHPad`.  Thanks!
> > This code moves the negate around within the local block
> 
> No?  It moves the negate into the same block as the definition of "V", which as far as I can tell is not the same block where the "sub" instruction was originally located.  (There's a dominance relationship, but that isn't really helpful here.)
> 
> In particular, the case I'm worried about is where "V" is a phi in the same basic block as a catchswitch.
Ack, you are right.  In the code where I observed the crash they were in the same block but it does not have to be so.  Will update.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D61089