[PATCH] D61089: [Reassociation] Place moved instructions after landing pads
David Greene via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Apr 30 13:37:13 PDT 2019
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;
----------------
greened wrote:
> 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.
Hrm. If we get into the situation where `V` is a PHI and there's a `catchswitch` right after it, what should we do? Just bail out of the loop and create a new `neg` before `BI`? That seems better than trying to replicate the `neg` at each destination of the `catchswitch`. I am not at all familiar with this code so I don't know what the implications are.
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D61089/new/
https://reviews.llvm.org/D61089
More information about the llvm-commits
mailing list