[polly] r252301 - Fix reuse of non-dominating synthesized value in subregion exit
Michael Kruse via llvm-commits
llvm-commits at lists.llvm.org
Sun Nov 8 20:30:15 PST 2015
2015-11-09 4:29 GMT+01:00 Johannes Doerfert <doerfert at cs.uni-saarland.de>:
>> > 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
Where do I get it?
> 1) Activate polly and unprofitable by default in your source. I have a
> separate commit to do that.
OK, I was using -opt-args=-polly-detect-unproditable, it it looks like
it is ignored.
> 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.
> 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.
I was creating and committing those before DeLICM on Tobias' request.
>> > 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.
> I don't get it. What rebase work? If you talk about different features
> that are ready or under construction and race with each other, that will
> not go away either way. Having a green bot at least allows to commit a
> new feature, though, the earlier we start commiting stuff the less
> rebase will be necessary at the end.
To be concrete here, if we are reverting r250625, you can start
committing new features, but I will have to rebase r250625 onto that
until finally we have DeLICM working.
Reordering code generation is a larger change and from previous such
cases we had, Tobias prefers to have them fixed even before structural
changes. From what I know the only thing broken at the moment is
DominatorTree preservation. It needs to be fixed anyway, why not now?
More information about the llvm-commits