[polly] r252301 - Fix reuse of non-dominating synthesized value in subregion exit

[Michael Kruse via llvm-commits]

2015-11-07 6:40 GMT+01:00 Johannes Doerfert <doerfert at cs.uni-saarland.de>:
>
> This actually caused 24 more failures, see:
>   http://lab.llvm.org:8011/builders/perf-x86_64-penryn-O3-polly-unprofitable/builds/557
>
> Before we were at 67, after at 91 and after r97986 we are now at 75,
> see:
>   http://lab.llvm.org:8011/builders/perf-x86_64-penryn-O3-polly-unprofitable/builds/569
>
> This brings me to several problems I would like to discuss:
>   1) Our "fixes" are not checked to be beneficial for more then the test
>      case that was attached to the bug report (see above).


I usually test my fixes on standard lnt (without
-polly-process-unprofitable) before committing.

>
>   2) The number of people looking at the lnt run and extracting tests is
>      to little. Tobias complained in SJ about that, now it just
>      shifted..

I may need some help on how to use bugpoint effectively. My reduced
cases are never as small as yours.


>   3) We try to fix things one by one while the underlying problem is not
>      exposed. Lately there are mainly dominance problems in non-affine
>      regions (we see the 4th or 5th bug now). While they are all fixed
>      by small changes to the code, it seems the changes always only
>      fix that test case and sometimes even make things worse (see 1)). I
>      would suggest to go back to the drawing board here and revert the
>      change that introduced these problems (changed the scalar
>      reload/store locations). Especially since we commited it
>      preemptively and now might not need it anyways. Please bring
>      forward counter arguments to this proposal.

For DeLICM by reusing array elements that are overwritten, lifetime
analysis needs to be done. In order to not have to analyze the
lifetime within statements, we need to ensure that scalars are only
written/read before/after the statement itself. Hence, we will still
need it (and at least some part of http://reviews.llvm.org/D13676).

Some of the bugs introduced by r250625 were just hidden before (like
the broken repair of the dominance tree) and others would just come up
again in other form which might be even harder to track because it
would miscompile instead of us getting a compile-time assertion fail
(r250625 was before we introduced the -polly-process-unprofitable
buildbot). Maintaining patches off-tree is also harder as it requires
regular rebasing.

>
>   4) We __all__ have to get our priorities straight. If these bots are
>      not green we cannot (or at least should not) commit anything new,
>      though there is already new stuff pending for a while now that
>      needs to be reviewed and decided on. The longer we wait the more
>      work we all have...

"Just" the rebase to the (supposed to be small) bugfix patches.
Rebasing to new features is harder. I feel this would just move the
rebase work from one person to another.

Michael