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

[Tom Rodriguez]

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

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

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

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.