Potential scheduling problem with compressed Oops.

[Tom Rodriguez]

Narrow oops can and do appear in oopmaps so I don't see anything wrong with that schedule.  Narrow oops that are live across a safepoint are handled by this code in BuildOopMaps:

    } else if( t->isa_narrowoop() ) {
      assert( !OptoReg::is_valid(_callees[reg]), "oop can't be callee save" );
      // Check for a legal reg name in the oopMap and bailout if it is not.
      if (!omap->legal_vm_reg_name(r)) {
        regalloc->C->record_method_not_compilable("illegal oopMap register name");
        continue;
      }
      if( mcall ) {
          // Outgoing argument GC mask responsibility belongs to the callee,
          // not the caller.  Inspect the inputs to the call, to see if
          // this live-range is one of them.
        uint cnt = mcall->tf()->domain()->cnt();
        uint j;
        for( j = TypeFunc::Parms; j < cnt; j++)
          if( mcall->in(j) == def )
            break;            // reaching def is an argument oop
        if( j < cnt )         // arg oops dont go in GC map
          continue;           // Continue on to the next register
      }
      omap->set_narrowoop(r);

So I think if you have a bug it's not with scheduling.  You need to figure out why the DecodeN wasn't added to the oopmap.

tom

On May 4, 2010, at 2:00 AM, Lindenmaier, Goetz wrote:

> Hi Tom, 
> 
> before scheduling, I saw something like this:
> 
>       LoadN
>         |  \
>         |   Safepoint
>         |  
>      DecodeN
> 	   |
>         |
> 	  AddP
> 		
> Where DecodeN defines a new register.
> Scheduling added a (dotted) precedence edge:
> 
>       LoadN
>         |  \
>         |   Safepoint
>         |       :
>      DecodeN    :
> 	   |      :
>         |  ....:   
> 	  AddP   
> 
> After scheduling, we had this:
> 
>       LoadN
>         |  \
>     DecodeN \
>         |    \  
>         |  Safepoint
> 	   |      :
>         |  ....:   
> 	  AddP   
> 
> And the register defined by DecodeN was not updated during a GC.
> 
> In my case, DecodeN and AddP were in a block succeeding the block with 
> the Safepoint, and our scheduler moved the Decode up, but I assume the
> same can happen within a single block.  
> I can't reproduce this with the Sun HotSpot, but wanted to point to 
> this potential problem.  
> 
> Cheers,
>  Goetz.
> 
> 
> 	
> -----Original Message-----
> From: Tom Rodriguez [mailto:tom.rodriguez at oracle.com] 
> Sent: Montag, 3. Mai 2010 19:30
> To: Lindenmaier, Goetz
> Cc: hotspot compiler
> Subject: Re: Potential scheduling problem with compressed Oops.
> 
> 
> On May 3, 2010, at 4:49 AM, Lindenmaier, Goetz wrote:
> 
>> Hi,
>> 
>> we are working on a port of HotSpot for ia64.  Currently, we 
>> enable this port to support compressed Oops.
>> 
>> I ran into an issue where a Decode node was moved above a safepoint
>> by the scheduler.  A precedence edge was missing to force the node 
>> behind the safepoint.
> 
> Can you describe in more detail why this causes a problem?  In general DecodeN and EncodeP can safely span safepoints as long as the anti-deps of their inputs are respected.
> 
> tom
> 
> 
>> This edge can easily be added if ComputeRegisterAntidependencies()
>> not only considers AddP in this code: 
>> 
>>    2627     if( n->jvms() ) {           // Precedence edge from derived to safept
>>    2628       // Check if last_safept_node was moved by pinch-point insertion in anti_do_use()
>>    2629       if( b->_nodes[last_safept] != last_safept_node ) {
>>    2630         last_safept = b->find_node(last_safept_node);
>>    2631       }
>>    2632       for( uint j=last_safept; j > i; j-- ) {
>>    2633         Node *mach = b->_nodes[j];
>>    2634         if( mach->is_Mach() && mach->as_Mach()->ideal_Opcode() == Op_AddP )
>>    2635           mach->add_prec( n );
>>    2636       }
>>    2637       last_safept = i;
>>    2638       last_safept_node = m;
>>    2639     }
>> 
>> So I changed the code here to test for Op_DecodeN and our problem 
>> disappeared.  
>> I want to share this information as I think this can happen on other 
>> platforms, too.  And it was pretty hard to track down.
>> We extended the scheduler to consider several blocks, increasing the
>> situations where nodes can be moved past Safepoints.  But I assume,
>> a suiting dataflow graph can appear on other platforms, too.  Or is 
>> there a constraint I missed?
>> 
>> Regards,
>> Goetz.
>> 
>> 
>> 
>> 
>> 
>> 
>> 
>> 
>> 
>