[polly] r244606 - Revise the simplification of regions

[Tobias Grosser via llvm-commits]

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.
  
>> 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).

Overall, I think this bug was rather easy to solve after Michael provided
some nice reduced test cases and a good analysis.

Best,
Tobias