[PATCH] Rename getMaximumUnrollFactor().

Renato Golin renato.golin at linaro.org
Tue Aug 26 13:03:59 PDT 2014


Hi Sanjay,

> In this version of the patch, I have changed the name to "getMaxVectorInterleaveFactor". I also made a bunch of interior changes in LoopVectorizer...but there are 2 "seams" where we still use "unroll":

Please, drop the "Vector" as it's not directly related to the interleaving.


> 1. I stopped short of changing the cl::opt names - if we really want to unify on "interleave" I assume we want to change those too so they match the pragmas? That will require changing 80+ test cases in test-suite. If everyone agrees that is better, I'll certainly make that change too.

That's a good point. We certainly could, but that'd have to be an alias, at least for now.

Arnold, how public are those flags nowadays? The decision to either keep it forever, rename now or anything in between will have to be based on that.


> 2. Eventually the interleave factor becomes an ingredient in "UF", the unroll factor:
>   if (UF > MaxInterleaveSize)
>     UF = MaxInterleaveSize;
> 
> I have not looked at the logic in LV close enough to know if "UF" is the appropriate name. Any opinions?

I think the name is correct as it is.

IIRC, UF is the actual unrolling factor, which bases its decision ultimately on the interleave factor, since it's generally not profitable to "just unroll" at a factor greater than the interleave, even if the data dependencies allow it.

cheers,
--renato

http://reviews.llvm.org/D5066






More information about the llvm-commits mailing list