[PATCH] D29641: [SLP] Fix for PR31847: Assertion failed: (isLoopInvariant(Operands[i], L) && "SCEVAddRecExpr operand is not loop-invariant!")

[Alexey Bataev via llvm-commits]

Michael,

1. It is quite hard to add a test case because we have a random crash. 
Reproducer from the PR is not crashed on my side anymore while still 
causes crashes for others. Should I add this test case?

2. SCEV is used during all of `vectorizeGEPIndices()`. But we can break 
SCEV during vectorizing of BasicBlock that is part of a loop. 
ScalarEvolution code has a cache of SCEV nodes that are not used 
anymore, but loop dispositions for these nodes are still active. This 
SCEV nodes may be reused later, for example, during some loop invariants 
folding. If folding is successful, we're asking for a new SCEV node for 
the fold result and get a SCEV node from cache that has its disposition 
`loop variant` rather than `loop invariant`. After that all SCEV 
operations may treat this SCEV node in the wrong way and it may crash 
the compiler (as it happens in this PR, when the following SCEV 
operations expect all SCEVs to be `loop invariant` while some of them 
are `loop variant` because of the wrong disposition of the cached SCEV 
node). To solve it we need to clear the info about loop dispositions of 
SCEV nodes to make ScalarEvolution to recalculate them. This is what is 
done by the ScalarEvolution code if some instructions are removed.

I talked with Sanjoy. He said that this may happen because of some 
LICM-like optimizations. SLP vectorizer may move code inside of the 
basic blocks too (instruction scheduler tries to bundle all vectorized 
instruction into a bunch, we're creating new instructions) and thus some 
of SCEV nodes are incorrectly invalidated.

-------------
Best regards,
Alexey Bataev

08.02.2017 0:24, Michael Kuperstein via Phabricator пишет:
> mkuper added a comment.
>
> Test case?
>
> Also, this looks a bit weird. Could you explain what exactly is going wrong?
> I'm asking because this somehow seems like it's the wrong granularity for this - after any change, you invalidate the disposition of the innermost loop that contains the basic block in which the root/seed lives. I'm not sure why this is the right thing to do.
>
>
> https://reviews.llvm.org/D29641
>
>
>