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

[Sanjin Sijaric]

Hi Andy,

I get the following with the additional pass:

      Unnamed pass: implement Pass::getPassName()
      Combine redundant instructions
      Unnamed pass: implement Pass::getPassName()
      Scalar Evolution Analysis  <-----
      ScalarEvolution-based Alias Analysis  <----
      SLP Vectorizer  <----
      Unnamed pass: implement Pass::getPassName()
      Simplify the CFG  

Without the pass:

      Combine redundant instructions
      Unnamed pass: implement Pass::getPassName()
      Scalar Evolution Analysis <------
      SLP Vectorizer <-----
      Unnamed pass: implement Pass::getPassName()
      Simplify the CFG

SCEV-AA preserves ScalarEvolution, so it won't get recomputed by the SLP vectorizer again.  Scev-aa should only get used from within the SLP vectorizer via "alias(...)" function, so it shouldn't result in too much overhead.

Thanks,
Sanjin

-----Original Message-----
From: Andrew Trick [mailto:atrick at apple.com] 
Sent: Wednesday, October 22, 2014 10:09 AM
To: Sanjin Sijaric
Cc: Hal Finkel; mcrosier at codeaurora.org; llvm commits; Arnold Schwaighofer
Subject: Re: [PATCH] Bug 21233 - Catch more cases in SLP vectorizer

Hi Sanjin,

Can you look at -debug-pass=Structure before and after your pass? Are there any differences. My understanding is that SCEV already runs before SLP vectorizer and this patch only enables it’s use in alias analysis, but I may be wrong about that. I also would like to think that the SLP vectorizer will only ask for SCEV/AA for a subset of memory ops. Will this change cause SCEV to be computed for all addresses?

What I’m getting at here is, could this be a compile time issue?

-Andy

> On Oct 22, 2014, at 8:49 AM, Sanjin Sijaric <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]
> Sent: Wednesday, October 22, 2014 8:08 AM
> To: mcrosier at codeaurora.org
> Cc: 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>
>> To: "Sanjin Sijaric" <ssijaric at codeaurora.org>
>> Cc: 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
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>> 
> 
> --
> Hal Finkel
> Assistant Computational Scientist
> Leadership Computing Facility
> Argonne National Laboratory
> <PATCH>