[PATCH] D60265: [LoopUnroll] Allow unrolling if the unrolled size does not exceed loop size.

[Florian Hahn via Phabricator via llvm-commits]

fhahn added a comment.

In D60265#1455080 <https://reviews.llvm.org/D60265#1455080>, @paquette wrote:

> LGTM. Just a couple little comments, but none of them are critical:
>
> - In D60266 <https://reviews.llvm.org/D60266> you have some code size numbers; it would be nice to have something similar here.


Right, I should have said something here. Without D60266 <https://reviews.llvm.org/D60266>, there was no change in code size on the set of benchmarks, most likely because loop-rotate is not run and they are not in the required form for unrolling to happen.

> - It would be nice to have a test with just the optsize attribute, since `optForSize()` will also return true if only that attribute is set.

I guess it would be sufficient to drop minsize from the attribute list, assuming minsize always come with optsize?

> - If the intention here is only -Oz/minsize, you might want to replace the call to `optForSize()` with `optForMinSize()`. This seems like an appropriate change for -Os/optsize though, so I don't think it's necessary.

Yep, I think it should be enabled for both -Os/-Oz.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60265/new/

https://reviews.llvm.org/D60265