[polly] r244606 - Revise the simplification of regions

[Johannes Doerfert via llvm-commits]

On 08/12, Tobias Grosser wrote:
> On 08/12/2015 09:38 AM, Johannes Doerfert wrote:
> >On 08/12, Michael Kruse via llvm-commits wrote:
> >>Two corrections:
> >>
> >>First, such PHI nodes can only happen in the entry node, since
> >>buildPHIAccesses checks the origin of the incoming edges (not of the
> >>operands which I assumed).
> >The operands are not the relevant part of the PHI semantics but the
> >control flow is. There are examples for this in the PHI codegen
> >discussion(s) we had on the list.
> 
> Agreed.
> 
> >>Sich PHI nodes can only appear in the entry
> >>node. So I think solutions 1 and 3 do work.
> >What is wrong with adding a pre SCoP block for all entry block PHI
> >nodes? That solved the problem before and I cannot see the negative side
> >effects and it needs only a minimal amount of code.
> 
> Right, that would also be a correct solution. I now just committed a
> slightly different fix in r244721, which seems equivalent simple.
> Maybe you can have a look. We can change the fix if another solution
> is clearly better.
I see two benefits and one drawback for the pre entry PHI block:
  + Less accesses in the SCoP (compile time)
  + Less dependences, hence restrictions, in the SCoP (runtime)
  - Not 100% accurate if Polly as an analyzer would also be required to
    give information about scalars (not only memory).

> >>This all doesn't look like designed deliberately. And it is what I
> >>meant with make simplifyRegion "stable" in D11867.
> >In the original PHI codegen patch there was a detection of PHI nodes
> >that were at some point part of the region (in the entry block) and
> >split out of it by one of the region simplification methods just before
> >the codegen part. Once detected these PHI's were deliberately ignored in
> >the block codegen as they were no longer part of the region (SCoP).
> >
> >I cannot argue for the simplification and splitting stuff but the PHI
> >codegen was aware of such PHI nodes and was handling them at some point.
> 
> To my understanding there was no need to handle them in the PHI codegen
> as they were always moved out of the scop before we actually did code
> generation. This is not the case any more, so we now model these kind of
> corner-case PHI nodes in r244721. I doubt the additional modeling has
> any cost and it seems indeed even simpler than a special PHI-node
> move function (which by itself would already be rather simple).
There was a check (and a comment) that PHI nodes could have been moved
out of the SCoP in which case we do not need to generate some code. I
don't know if it's still there but it was at some point.

> Overall, I think this bug was rather easy to solve after Michael provided
> some nice reduced test cases and a good analysis.
Agreed, but the analysis did not contain the initial (and for me
trivial) solution that would not have changed the behaviour of Polly
with regards to entry block PHI nodes at all.

Cheers,
  Johannes

-- 

Johannes Doerfert
Researcher / PhD Student

Compiler Design Lab (Prof. Hack)
Saarland University, Computer Science
Building E1.3, Room 4.31

Tel. +49 (0)681 302-57521 : doerfert at cs.uni-saarland.de
Fax. +49 (0)681 302-3065  : http://www.cdl.uni-saarland.de/people/doerfert
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 213 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150812/951ab8a5/attachment.sig>