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

[Vladimir Kozlov]

I looked which nodes are pinned and all of them are CFG nodes
except Phi and SafePointScalarObject.
The block projection control edge of CFG nodes will be replaced
with new Region nodes during PhaseCFG::build_cfg() before GCM.
And Phi node can not have block projection control - only Region.
Which leaves only SafePointScalarObject node we need to patch.

I can add additional assert to verify this:

  void PhaseCFG::replace_block_proj_ctrl( Node *n ) {
    const Node *in0 = n->in(0);
    assert(in0 != NULL, "Only control-dependent");
    const Node *p = in0->is_block_proj();
    if (p != NULL && p != n) {    // Control from a block projection?
+    assert(!n->pinned() || n->is_SafePointScalarObject(), "not SafePointScalarObject");
      // Find trailing Region

Vladimir

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