[PATCH] D21720: Unroll for uncountable loops

Hal Finkel via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Mar 2 17:03:00 PST 2017


hfinkel added a comment.

In https://reviews.llvm.org/D21720#691221, @evstupac wrote:

> Hi Michael,
>
> >> but we shouldn't forget about the increased source code complexity.
>
> The patch mostly reuse existing code for full unroll.
>
> Regarding PHI Cycle it even improve current heuristic, as full unroll consider
>  "s^=1;" is constant "1" at all iterations >0. However it is 0 on even and 1 on odd.
>  And I'm ok to drop this part to make code shorter.
>
> Maybe we can add uncountable loops unroll under -O3?


I agree. -O3 would be the right place for this.

> Thanks,
> Evgeny



In https://reviews.llvm.org/D21720#691193, @mzolotukhin wrote:

> Hi,
>
> I'm still not convinced that we need this functionality. The loops that you mentioned can easily be handled by using unroll pragma if users really care about their performance. The patch might introduce no performance/compile-time regressions, but we shouldn't forget about the increased source code complexity.
>
> Michael


I don't think that "easily be handled by using unroll pragma" is the right standard here. The same argument can be made for anything. We should only really require a pragma when we cannot design a reasonable heuristic (at this stage in the pipeline). This patch does not seem that large. I can review next week.


Repository:
  rL LLVM

https://reviews.llvm.org/D21720





More information about the llvm-commits mailing list