Request for reviews (S): 6809798: SafePointScalarObject node placed into incorrect block during GCM

[Tom Rodriguez]

Ok.

Since you've split the processing into a new method would you remove  
the copy of the comment about "Special control input processing" from  
the original location?  Maybe a shorter comment like this:

// Fixup some control.  Constants without control get attached to root  
and
// nodes that use is_block_proj() nodes should be attached to the  
region that
// starts their block.

Actually the existing comment for replace_block_proj_ctrl is pretty  
hard to read.  How about:

// Nodes that have is_block_proj() nodes as their control need to use
// the appropriate Region for their actual block as their control since
// the projection will be in a predecessor block.

tom

On Feb 26, 2009, at 11:13 AM, Vladimir Kozlov wrote:

> I only saw asserts for SafePointScalarObjectNodes.
> But patching control edge of pinned nodes before scheduling
> affects all pinned nodes: safepoints, phis.
> And I think this change is correct for all pinned nodes.
>
> I actually was not able to catch the case were
> safepoint has block projection control to see what
> happened. In the bugs case it was scheduled correctly
> even with current code. But it could be the case
> when control was set differently for SFPT and SPSO nodes
> during IGVN in IfNode::dominated_by() which I fixed by
> SafePointScalarObjectNode::depends_only_on_test() const { return  
> false; }
>
> Thanks,
> Vladimir
>
> Tom Rodriguez wrote:
>> Were SafePointScalarObjectNodes the only one affected by this or  
>> will this change the schedule of other nodes?
>> tom
>> On Feb 26, 2009, at 10:15 AM, Vladimir Kozlov wrote:
>>> http://cr.openjdk.java.net/~kvn/6809798/webrev.00
>>>
>>> Fixed 6809798: SafePointScalarObject node placed into incorrect  
>>> block during GCM
>>>
>>> Problem:
>>> The main problem is that the control edge of a pinned node
>>> could be replaced during schedule_early after the pinned
>>> node scheduled into a block if the control node is a block
>>> projection.
>>> Also there were few places where the control edge of
>>> a SafePointScalarObject node was not adjusted correctly.
>>>
>>> Solution:
>>> Replace the control edge of a pinned node before scheduling.
>>> Adjust the control edge of a SafePointScalarObject when needed.
>>> Added few asserts to verify scheduling correctness for
>>> pinned nodes and SafePointScalarObject node.
>>>
>>> Reviewed by:
>>>
>>> Fix verified (y/n): y
>>>
>>> Other testing:
>>> JPRT, CTW (with -XX:+DoEscapeAnalysis)
>>>