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

[Tobias Grosser via llvm-commits]

On 11/09/2015 04:29 AM, Johannes Doerfert via llvm-commits wrote:
> On 11/09, Michael Kruse wrote:
>> 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.
> I do the same thing, usually. However, as we are still debugging the
> unprofitable bot we have to test it with unprofitable activated,
> otherwise we will not be informed if we broke something there (which is
> the only interesting information we will get from that bot).
>
> I found the problem described above by chance and searched for the
> commit that increased the number of broken tests manually...
>
>>>    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.
> -1) Configure polly such that it is linked in all tools.
>   0) Apply Tobias bugpoint patch (I haven't but it works ok most of the
>      times).

Attached. It is not always needed, but sometimes helps to drastically 
remove unrelated control flow,

>   1) Activate polly and unprofitable by default in your source. I have a
>      separate commit to do that.
>   2) Create a plain IR file from your input (e.g., remove the -O3 from the
>      lnt run line and add -emit-llvm). Now your output file is a bitcode file
>      that we will simplify.

Use:

-mllvm -polly=false -S -emit-llvm  -disable-llvm-optzns -o out.ll

Just dropping -O3 makes LLVM to also not emit type-based alias 
information, which is sometimes needed.

>   3) run "bugpoint -O3 <file>" and hope the result will crash with the
>      same kind of error and is small enough to understand.
>
>>>    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.
> Let's not go in the DeLICM discussion again here. If we at some point
> commit something that needs this change, we can apply it, but at the
> moment it just makes our lives harder. A new codegen structure might
> solve the problem I was describing here too, namely that we apply these
> local patches to fix one corner case and at some point loose the
> structure in our code which will cause new corner cases to appear.

Best,
Tobias
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Reduce-control-flow-edges-in-bugpoint.patch
Type: text/x-patch
Size: 9772 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151109/f6630d57/attachment.bin>