[PATCH] Bug 21233 - Catch more cases in SLP vectorizer

Andrew Trick atrick at apple.com
Wed Oct 22 10:23:16 PDT 2014 

> On Oct 22, 2014, at 10:07 AM, Arnold Schwaighofer <aschwaighofer at apple.com> wrote:
> 
> I just talked to Andy and he explained to me that the philosphy is for SCEV to be the canonical form. While indvars should not actively obfuscating code optimizations must not rely on a pass putting the IR into a canonical form.
> 
> This clearly argues for using SCEV. Downside is the compile time cost of building scev - which is not conserved by passes.

Arnold is right that something that uses SCEVExpander is obfuscating the IR. It would be nice to know what. Is it the partial unroller? (I really don’t like that pass running independently from the loop vectorizer).

There is another completely different way to approach this. The SLP vectorizer wants to know if instructions can statically alias within the same block. AliasAnalysis is way to conservative to handle this query. In other words, we should have a kind of alias analysis that can treat phis (especially in the loop header) as base pointers.

-Andy

> Thanks,
> Arnold
> 
> 
>> On Oct 22, 2014, at 9:35 AM, Arnold Schwaighofer <aschwaighofer at apple.com <mailto:aschwaighofer at apple.com>> wrote:
>> 
>> Is using a heavier analysis the right answer here? How did we end up with this IR in the first place (i suspect indvars - was the IR easier to analyse before running it? - if the answer is yes - did it not do its job properly? can we fix that?).
>> 
>> +loop_header:                                ; preds = %loop_header, %entry
>> +  %scevgep.phi = phi i32* [ %dst, %entry ], [ %scevgep.inc, %loop_header ]
>> +  %indvar = phi i64 [ 0, %entry ], [ %indvar_next, %loop_header ]
>> +  %1 = shl i64 %indvar, 4
>> +  store i32 %value, i32* %scevgep.phi, align 4                                 <=== this store uses the scev.gep.phi, could we rewrite it to also index of %dst and %1.
>> +  %scevgep48.sum78 = or i64 %1, 1
>> +  %scevgep49 = getelementptr i32* %dst, i64 %scevgep48.sum78 <=== the follow up stores compute off the input of the phi
>> +  store i32 %value, i32* %scevgep49, align 4
>> +  %scevgep50.sum79 = or i64 %1, 2
>> +  %scevgep51 = getelementptr i32* %dst, i64 %scevgep50.sum79
>> +  store i32 %value, i32* %scevgep51, align 4
>> +  %scevgep52.sum80 = or i64 %1, 3
>> +  %scevgep53 = getelementptr i32* %dst, i64 %scevgep52.sum80
>> +  store i32 %value, i32* %scevgep53, align 4
>> +  %indvar_next = add nsw i64 %indvar, 1
>> +  %exitcond = icmp eq i64 %indvar_next, %0
>> +  %scevgep.inc = getelementptr i32* %scevgep.phi, i64 16
>> +  br i1 %exitcond, label %loop_exit, label %loop_header
>> +}
>> 
>> 
>> Thanks,
>> Arnold
>> 
>>> On Oct 22, 2014, at 8:49 AM, Sanjin Sijaric <ssijaric at codeaurora.org <mailto:ssijaric at codeaurora.org>> wrote:
>>> 
>>> Hi Chad and Hal,
>>> 
>>> Thanks for catching this, Chad.  It was not intentional.  The new patch is attached.
>>> 
>>> Sanjin
>>> 
>>> -----Original Message-----
>>> From: Hal Finkel [mailto:hfinkel at anl.gov <mailto:hfinkel at anl.gov>] 
>>> Sent: Wednesday, October 22, 2014 8:08 AM
>>> To: mcrosier at codeaurora.org <mailto:mcrosier at codeaurora.org>
>>> Cc: llvm-commits at cs.uiuc.edu <mailto:llvm-commits at cs.uiuc.edu>; Sanjin Sijaric; Andrew Trick; Arnold Schwaighofer
>>> Subject: Re: [PATCH] Bug 21233 - Catch more cases in SLP vectorizer
>>> 
>>> ----- Original Message -----
>>>> From: "Chad Rosier" <mcrosier at codeaurora.org <mailto:mcrosier at codeaurora.org>>
>>>> To: "Sanjin Sijaric" <ssijaric at codeaurora.org <mailto:ssijaric at codeaurora.org>>
>>>> Cc: llvm-commits at cs.uiuc.edu <mailto:llvm-commits at cs.uiuc.edu>
>>>> Sent: Wednesday, October 22, 2014 9:57:36 AM
>>>> Subject: Re: [PATCH] Bug 21233 - Catch more cases in SLP vectorizer
>>>> 
>>>> Hi Sanjin,
>>>> 
>>>>> The attached patch addresses bug #21233.  I didn't see a way to 
>>>>> vectorize the attached test case without using SCEV-AA along with a 
>>>>> small change in SLP vectorizer to retrieve the base pointer using 
>>>>> SCEV analysis.
>>>> 
>>>> +      if (UseSCEVAAForSLP)
>>>> +        MPM.add(createScalarEvolutionAliasAnalysisPass());
>>>> +      MPM.add(createSLPVectorizerPass());   // Vectorize parallel
>>>> scalar
>>>> chains.
>>>> 
>>>> This looks to be add a call to the SLP vectorizer pass.  Is this 
>>>> change indented?  I imagine you just want to add the SCEV-AA pass 
>>>> before the existing SLP vectorizer pass a few lines above this change.
>>>> 
>>>>> Can someone please have a look?  Is it safe to use SCEV-AA, or 
>>>>> should I pursue a different approach?
>>>> 
>>>> I'm not the right person to comment on the use of SCEV-AA in the SLP 
>>>> vectorizer.  Perhaps, Arnold, Hal, or someone else could provide their 
>>>> opinion.
>>> 
>>> We'd obviously not been using SCEV-AA because the current pass manager does not allow it to be preserved. As a result, I'm not sure how well tested it is (it is also pretty simple, so I'm not super concerned, only somewhat concerned). My thought has been that, once we have the new pass manager, we can do a cost-benefit analysis on using SCEV-AA more generally (especially since doing so might allow us to strip down BasicAA somewhat). I'm not opposed to gaining some more experience with SCEV-AA by using it in a targeted place like this.
>>> 
>>> Andy, do you have an opinion on this? Arnold?
>>> 
>>> -Hal
>>> 
>>>> 
>>>> Chad
>>>> 
>>>> _______________________________________________
>>>> llvm-commits mailing list
>>>> llvm-commits at cs.uiuc.edu <mailto:llvm-commits at cs.uiuc.edu>
>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>> 
>>> 
>>> --
>>> Hal Finkel
>>> Assistant Computational Scientist
>>> Leadership Computing Facility
>>> Argonne National Laboratory
>>> <PATCH>
>> 
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu <mailto:llvm-commits at cs.uiuc.edu>
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits <http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20141022/cd039976/attachment.html>

More information about the llvm-commits mailing list