[PATCH] Reimplement heuristic for estimating complete-unroll optimization effects.

Chandler Carruth chandlerc at gmail.com
Tue May 12 09:11:55 PDT 2015


Generally, I think this can go in. There are a bunch of things I think should be cleaned up here, but they're fairly minor and I'm happy to just fix those and for a few that have more impact, send you patches.

In http://reviews.llvm.org/D8816#170783, @mzolotukhin wrote:

> Hi Chandler,
>
> Thanks for the comments! I believe I've addressed in the source all of them, except this one:
>
> > Is there a reason you don't make visit() return a bool indicating whether it's cost should be counted or not, and localize all the counting in this function? It would be much easier to understand IMO.
>
> > 
>
> > I think I would also find it easier to read this as counting the number of instructions that will actually result from unrolling (essentially, the *un*optimized instructions) and the optimized instructions. You could still sum them and divide to compute the percentage, but it would make the threshold check not require subtraction. That could be done in a follow-up patch though.
>
>
> I kept counting the cost inside the `visit` function, because we might have three cases there:
>
> 1. instruction was simplified to a constant (e.g. x = a[0] * y = 0 * y = 0)
> 2. instruction was simplified, but not to a constant (e.g. x = a[0] + y = 0 + y = y)
> 3. instruction wasn't simplified
>
>   In case we want to distinguish (1) and (2) outside the `visit` function, `bool` won't be enough. Currently we won't lose much by merging them though, but I didn't want to limit ourselves here from the very beginning.


I don't think we need to distinguish between them. The key to realize is that if 'y' above were inside the loop body, we would already have accounted for its cost. The critical thing is whether we can fold 'x' away.

While perhaps we'll want more advanced heuristics, but I would rather assume not and simplify the code accordingly until a real use case arrives. (YAGNI, essentially.)

If you agree, I'm happy to make this change.

The only specific change I'd like to request you make in a follow-up are to ensure some of the test cases you mentioned in email exercising the percentage threshold etc are actually checked in as test cases.

Thanks!


http://reviews.llvm.org/D8816

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list