[PATCH] [LoopVectorize] Split out LoopAccessAnalysis from LoopVectorizationLegality

[Hal Finkel]

----- Original Message -----
> From: "Adam Nemet" <anemet at apple.com>
> To: anemet at apple.com, hfinkel at anl.gov, aschwaighofer at apple.com
> Cc: llvm-commits at cs.uiuc.edu
> Sent: Friday, January 30, 2015 12:16:00 PM
> Subject: Re: [PATCH] [LoopVectorize] Split out LoopAccessAnalysis from LoopVectorizationLegality
> 
> ================
> Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:613
> @@ +612,3 @@
> +/// This class is responsible for analyzing the memory accesses of a
> loop either
> +/// at compile or at run-time.  It collects the accesses and then
> its main
> +/// helper the AccessAnalysis class finds and categorizes the
> dependences in
> ----------------
> hfinkel wrote:
> > This class does not do anything "at run-time" ;)
> Sure, I'll reword it.
> 
> ================
> Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:1800
> @@ -1744,3 +1799,3 @@
>  
> -bool LoopVectorizationLegality::isUniform(Value *V) {
> +bool LoopAccessAnalysis::isUniform(Value *V) {
>    return (SE->isLoopInvariant(SE->getSCEV(V), TheLoop));
> ----------------
> hfinkel wrote:
> > Can you please group together the implementations of the functions
> > for each class? I can see some advantage to having these two near
> > each other, but in the end, I think having the implementation
> > spread out over different parts of the file is confusing.
> > 
> Sure.  I just kept the original order so that it's immediately
> apparent what changed in the diff.  Sometime these things show up as
> code movement with modifications which are harder to figure out.
> 
> What do you think about adjusting the order in an additional step in
> the patchset?

Sure. Just make sure you get them all when you move them into the new file (I think some of these out-of-order ones may have been missed in the later patchset).

 -Hal

> 
> http://reviews.llvm.org/D7282
> 
> EMAIL PREFERENCES
>   http://reviews.llvm.org/settings/panel/emailpreferences/
> 
> 
> 

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory