RFR: 8184795: [MVT] non constant method handles can cause allocations in method handle combinators

[Tobias Hartmann]

Hi Roland,

On 19.07.2017 15:17, Roland Westrelin wrote:
>> src/share/vm/opto/castnode.cpp
>> - It would be nice to move the allocation code in
>> CheckCastPPNode::Ideal to valuetype.cpp or merge it with
>> ValueTypeNode::allocate
> 
> Ok. Makes sense. Merging with ValueTypeNode::allocate doesn't seem
> reasonable though because that one only runs during parsing.

Right, just would have been nice to reuse the code in GraphKit::new_instance instead of creating the allocate/initialize/barrier nodes manually.

>> src/share/vm/opto/cfgnode.cpp
>> - In line 1657, shouldn't this be:
>>   assert(in(i) == NULL || (vtptr != NULL && phase->type(in(i))->higher_equal(vtptr)) || [...]
> 
> vtptr can be null if the first non null phi input is not a ValueTypePtr.

Okay, I missed that we don't bail out then (but we could).

>> - Why do you need the (in(1)->is_Phi()) check? Maybe add { } and a comment
> 
> The new allocation reuses the exception paths from the call above
> it with extra regions and phis. I found that would break that logic.

Okay, makes sense.

>> src/share/vm/opto/type.cpp
>> - Why did you change this from TypeKlassPtr to TypePtr?
> 
> Because we're returning either a class pointer or a pointer to a
> value. So it's not always a TypeKlassPtr.

Right, missed that.

>> Was this problem triggered by one of the tests? I'm seeing spurious
>> failures on JPRT (without your changes) and wonder if that could be
>> the cause.
> 
> It wasn't. I don't think it exists without that change.

Okay, I'll try to figure out what the problem is with these failures. I'm not able to reproduce them locally.

Best regards,
Tobias