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

[Alexey Bataev via Phabricator via llvm-commits]

ABataev added a comment.

In https://reviews.llvm.org/D29641#728047, @Gerolf wrote:

> Hi,
>
> I would like to understand this better. Could somebody explain what assumptions SCEV makes about its clients? Which assumption(s) is broken by SLP? It seems to me that this issue potentially touches fundamental design decisions/questions and I don't see any verifiers in place.
>
> Thanks
> Gerolf


Hi Gerolf, sorry for the delay with the answer.
I suppose Sanjoy answered your question already:

> You can't replace the operands of an instruction in a way that it computes a new value while still preserving SCEV, since SCEV keys off of the Instruction *. Loop disposition is only one of the things that this breaks. e.g. if you have 1 + %KnownPositive and you replace the %KnownPositive operand with %MayBeNegative ("in place"), that breaks the getRange cache.
> 
> Can the loop vectorizer do without replacing %cond and %cond14 to undef?

So, I think you can't replace instructions with one disposition by the instruction with another disposition. You can just remove it.



================
Comment at: test/Transforms/SLPVectorizer/X86/crash-SCEV.ll:1
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt -slp-vectorizer -S -o - -mtriple=i386 -mcpu=haswell < %s | FileCheck %s
----------------
Gerolf wrote:
> What is this supposed to test? I'm wondering if there can be a smaller test for what this one is supposed to do. As is at the least it looks hard to maintain.
This is a reproducer, that currently crashes the compiler for sure. I tried to make it as small as possible, but could not reduce it anymore.


https://reviews.llvm.org/D29641