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

[Tobias Grosser]

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?

Tobi