Request reviews (S): 6896352: CTW fails hotspot/src/share/vm/opto/escape.cpp:1155

[Vladimir Kozlov]

Yes, it works currently without volatile because of side effect
but I did not feel comfortable with the code as it looks.
May be I should just add the comment that C++ will not remove
the call since it has side effect.

The code in escape.cpp creates new alias type with instance id
which is different from original general type created now in
Ideal_common (and in different places before).
When we process allocations candidates for scalar replacement
we cast their type (type of CheckCastPP node) to instance id:

       const TypeOopPtr *t = igvn->type(n)->isa_oopptr();
       if (t == NULL)
         continue;  // not a TypeInstPtr
       tinst = t->cast_to_exactness(true)->is_oopptr()->cast_to_instance_id(ni);

Than we pass the allocation (CheckCastPP node) node as base to
split_AddP() and replace AddP node's type with new one:

   const TypeOopPtr *base_t = igvn->type(base)->isa_oopptr();
...
   const TypeOopPtr *tinst = base_t->add_offset(t->offset())->is_oopptr();
   // Do NOT remove the next line: ensure a new alias index is allocated
   // for the instance type
   int alias_idx = _compile->get_alias_index(tinst);
   igvn->set_type(addp, tinst);

This is the core for scalar replacement and how we separate memory
slices for non-escaping objects which we intend to eliminate.

Vladimir

Tom Rodriguez wrote:
> I don't think adding volatile could have the effect you are hoping for 
> and I also don't think it's needed even if it did.  get_alias_index has 
> a side effect over in find_alias_type so it's impossible for it to be 
> optimized away even if the return value isn't used.  Also if the new 
> call in Ideal_common is doing it's job then do you really need the call 
> in escape.cpp?  I don't see how splitting an AddP could create a new 
> alias type if one already existed for that AddP.
> 
> tom
> 
> On Nov 4, 2009, at 10:51 AM, Vladimir Kozlov wrote:
> 
>>
>> http://cr.openjdk.java.net/~kvn/6896352/webrev.00
>>
>> Fixed 6896352: CTW fails hotspot/src/share/vm/opto/escape.cpp:1155
>>
>> Problem:
>> Alias type for LoadUS(ConP char[]) node from the test
>> was not defined during Parse before EA since
>> C->get_alias_index(phase->type(address)) was not called.
>> But EA expects all alias types to be defined before it starts.
>>
>> Solution:
>> Always call C->get_alias_index(phase->type(address)) in
>> MemNode::Ideal_common() which is called by all memory nodes.
>> I added "volatile" to expressions to avoid C++ removal
>> since the result is not used.
>>
>> Reviewed by:
>>
>> Fix verified (y/n): y, test
>>
>> Other testing:
>> JPRT, CTW
>>
>