[PATCH] [LoopVectorize] Split out LoopAccessAnalysis from LoopVectorizationLegality

[Adam Nemet]

> On Jan 30, 2015, at 10:24 AM, Hal Finkel <hfinkel at anl.gov> wrote:
> 
> ----- 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).

Yes, thanks for noticing.  I think now I didn’t miss anything.

Also there was no need to reorder since everything that was renamed to LAA ends up being moved to the new module so LoopVectorizationLegality implementation is contiguous after that.

Adam  

> -Hal
> 
>> 
>> http://reviews.llvm.org/D7282 <http://reviews.llvm.org/D7282>
>> 
>> EMAIL PREFERENCES
>>  http://reviews.llvm.org/settings/panel/emailpreferences/ <http://reviews.llvm.org/settings/panel/emailpreferences/>
>> 
>> 
>> 
> 
> -- 
> Hal Finkel
> Assistant Computational Scientist
> Leadership Computing Facility
> Argonne National Laboratory

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