[PATCH] D30350: [LSR] Add a cap for reassociation of AllFixupsOutsideLoop type LSRUse to protect compile time

[Wei Mi via llvm-commits]

On Fri, Mar 3, 2017 at 10:45 PM, Sanjoy Das via Phabricator <
reviews at reviews.llvm.org> wrote:

> sanjoy added inline comments.
>
>
> ================
> Comment at: lib/Transforms/Scalar/LoopStrengthReduce.cpp:3430
> +  if (AddOps.size() >= 5 && LU.AllFixupsOutsideLoop)
> +    return;
> +
> ----------------
> I'm not too familiar with LSR, but this looks pretty ad-hoc -- why can't
> the same compile-time blowup happen for an `LSRUse` with
> `AllFixupsOutsideLoop` = `false`?
>
>
In theory reassocation can also cause candidate formulae to blow up for
LSRUse with AllFixupsOutsideLoop = false. I only handle LSRUses with
AllFixupsOutsideLoop = true because they should have less impact on the the
final LSR result when we put a reassociation cap.

Because it is not a common problem and we havn't seen real case involving
LSRUse with AllFixupsOutsideLoop = false suffering from it, I choose to
limit the range affected by the change.

Thanks,
Wei.


>
> Repository:
>   rL LLVM
>
> https://reviews.llvm.org/D30350
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170306/e1a90f13/attachment.html>