Request for reviews (S): 6959430: Make sure raw loads have control edge

[Vladimir Kozlov]

Tom Rodriguez wrote:
> Looking at the webrev shows the problem with requiring control on every raw load.  These loads:
> 
> !   Node* threadObj = make_load(NULL, p, thread_type, T_OBJECT);
> !   Node* threadObj = make_load(control(), p, thread_type, T_OBJECT);

I did it because threadObj could be moved during GC/safepoint (thread.cpp):

   // Traverse instance variables at the end since the GC may be moving things
   // around using this function
   f->do_oop((oop*) &_threadObj);

> 
> !   Node* osthread = make_load(NULL, p, TypeRawPtr::NOTNULL, T_ADDRESS);
> !   Node* osthread = make_load(control(), p, TypeRawPtr::NOTNULL, T_ADDRESS);

I agree that this may not need control.

> 
> will no longer common with their equivalents since they will all have control.  Those loads will never need control because their value cannot change.  Actually it would work better if they were using immutable_memory as well.  We really need to split out the raw alias category into more classes.  Maybe we need to allow no control in a few cases?  We could capture it in a helper somewhere.

> 
> Why did you change the memory here?
> 
> !   Node* n3 = new(C, 3) LoadINode(NULL, immutable_memory(), p3, TypeRawPtr::BOTTOM);
> !   Node* n3 = new(C, 3) LoadINode(NULL, memory(p3), p3, _gvn.type(p3)->is_ptr());

I copied code from GraphKit::gen_subtype_check() to avoid using RAW ptr.

Vladimir

> 
> tom
> 
> On Jun 8, 2010, at 7:12 PM, Vladimir Kozlov wrote:
> 
>> I will push it after 6953058 fix is pulled into our repository.
>>
>> http://cr.openjdk.java.net/~kvn/6959430/webrev
>>
>> Fixed 6959430: Make sure raw loads have control edge
>>
>> Safepoint polls don't produce a new raw memory state
>> so loads of raw are allowed to float above safepoint
>> during which a raw value could be modified.
>>
>> Note, I added two checks. One is in factory methods
>> so the call stack will show where it is called from.
>> An other is in final graph reshape code to catch
>> nodes for which constructors were used directly.
>>
>> Passed CTW and JPRT.
>