[llvm-dev] [SCEV][ScalarEvolution] SE limitation impacting LV

[Caballero, Diego via llvm-dev]

Ok. Thanks a lot for the feedback!

>> You probably should fix the vectorizer to avoid generating PHI nodes like %18;
>> even if the vectorizer itself doesn't care if SCEV can analyze the remainder loop,
>> it matters to later passes like LSR.

I agree. As Ayal and Gil mentioned in D39346, even the vectorizer might care if SCEV can't analyze the remainder loop if we eventually want to vectorize it.
I'll report this issue.

Thanks,
Diego

-----Original Message-----
From: Sanjoy Das [mailto:sanjoy at playingwithpointers.com] 
Sent: Thursday, November 23, 2017 12:04 PM
To: Friedman, Eli <efriedma at codeaurora.org>
Cc: Caballero, Diego <diego.caballero at intel.com>; llvm-dev at lists.llvm.org
Subject: Re: [llvm-dev] [SCEV][ScalarEvolution] SE limitation impacting LV

On Wed, Nov 22, 2017 at 4:23 PM, Friedman, Eli <efriedma at codeaurora.org> wrote:
> The PHI node %18 is equivalent to a recurrence, so in theory SCEV 
> could analyze it... but it would be complicated to prove that.  
> Specifically, it would have to prove %17 - 1 == %18, which means 
> proving the relationship between %bc.resume.val and %bc.resume.val1.  
> There isn't any code in SCEV to do that sort of analysis.

+1

>>         > SCEV is fairly conservative around PHI nodes that aren't 
>> recurrences and aren't obviously equivalent to a min-max branch-phi idiom.
>> Is that the limitation you're running into here?
>>   Yes, that's exactly the problem. The problematic PHI nodes 
>> (%bc.resume.val and %bc.resume.val1) aren't either recurrences or 
>> related to min-max idioms. I have a tentative fix for the LV bug and 
>> I just wanted to know if this is something that should also be fixed 
>> at SCEV level to report a bug accordingly. Your previous comment 
>> sounds like "this is a well-known limitation in SCEV" so maybe there is nothing new to report.

Yes.

However, feel free to send patches if there are specific cases you want SCEV to handle (design-wise a limited amount of ad-hoc logic here seems fine to me:
https://github.com/llvm-mirror/llvm/blob/master/lib/Analysis/ScalarEvolution.cpp#L5072).

-- Sanjoy