[PATCH] D73501: [SCEV] rewriteLoopExitValues(): even if have hard uses, still rewrite if cheap (PR44668)

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added subscribers: danilaml, bjope, srking.
lebedev.ri added a comment.

In D73501#1905171 <https://reviews.llvm.org/D73501#1905171>, @samparker wrote:

> Hi,


Hi. Thank you for bringing this up to my attention.

> We track the performance of our (Arm) open source DSP library
>  and the cost model fixes were generally a notable improvement,
>  so many thanks for that!

Which i believe means that we stopped performing many of the expansions
we've previously done because they now no longer fit
into our arbitrarily-picked budget magic number..

> But the final patch for rewriting exit values has generally been bad,
>  especially considering the gains from the modelling improvements.

The goal of this change in particular was to really try to get rid of
uses of IV outside of the loop, since that was one of the obstacles
preventing vectorization in one of the loops i looked at.
(read: in some cases this patch could lead to dramatic improvements due to vectorization)

In light of cost modelling issues you pointed out, this means two things:

1. We need an undo transform :) It obviously would need to be quite late in pipeline though. https://bugs.llvm.org/show_bug.cgi?id=42965 / D12494 <https://reviews.llvm.org/D12494> / D66450 <https://reviews.llvm.org/D66450> (CC @reames @danilaml @srking - what would it take such a pass moving? :))
2. After all, we might not be modelling the cost correctly, as pointed out in post-commit review notes @ D73744 <https://reviews.llvm.org/D73744>. Will take a look.

> I need to look into it further, but on my current test case I'm seeing +30% increase
>  in stack accesses with a similar decrease in performance.

I'm not sure what "increase in stack accesses" mean here?
I'm reading that as: since we need some values after the loop (to re-compute the values),
we end up spilling them onto stack to free up registers for/in loop,
and then end up reloading them after the loop? That's not unexpected i guess,
although the perf drop (define "similar"?) is obviously alarming.

I'm interested whether @dmgreen, @bjope also observed something similar from these patches?

> Did you get performance numbers for these patches?
>  I'm just wondering if you observed any negative effects yourself?

I just double-checked, and i'm not seeing anything like that.
But cost modelling being cost modelling i'm not very surprised.
Let's see what happens if i address post-commit-review comments..

On LLVM's LNT, the closest runs are http://lnt.llvm.org/db_default/v4/nts/132695?compare_to=132694
It says there are some improvements and one regression,
but from http://lnt.llvm.org/db_default/v4/nts/graph?plot.0=1364.1604571.3&highlight_run=132695
that regression looks like noise to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73501