[llvm-commits] [llvm] r154007 - /llvm/trunk/lib/Transforms/Scalar/LoopUnrollPass.cpp

[Hongbin Zheng]

Hi tobi,

On Wed, Apr 4, 2012 at 8:10 PM, Tobias Grosser <tobias at grosser.es> wrote:
> On 04/04/2012 02:07 PM, Hongbin Zheng wrote:
>>
>> Hi tobi,
>>
>> It looks like a trivial typo [1],  as you can find the comment of
>> "Threshold":
>>
>>   // Determine the current unrolling threshold.  While this is normally
>> set
>>   // from UnrollThreshold, it is overridden to a smaller value if the
>> current
>>   // function is marked as optimize-for-size, and the unroll threshold was
>>   // not user specified.
>>   unsigned Threshold = CurrentThreshold;
>>   if (!UserThreshold&&
>>       Header->getParent()->hasFnAttr(Attribute::OptimizeForSize))
>>     Threshold = OptSizeUnrollThreshold;
>>
>> The "Threshold" is overridden by the the value of
>> "OptSizeUnrollThreshold" if the function requires to be optimized for
>> size. Hence, when we are reducing the unroll count of the loop to meet
>> the threshold constraint, the original code reduce the unroll count
>> according the variable "CurrentThreshold", instead of "Threshold",
>> which may be adjusted by the function attribute.
>>
>> The original code:
>>       if (TripCount) {
>>         // Reduce unroll count to be modulo of TripCount for partial
>> unrolling
>>         Count = CurrentThreshold / LoopSize;
>> <--------------------------should use Threshold
>>         while (Count != 0&&  TripCount%Count != 0)
>
>
> Sure. But maybe still worth adding a test case that stops this bug to be
> reintroduced in the next restructuring. Would it be too hard to create one?
Nope, i am going to do this :)

best regards
ether
>
> Tobi