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

Tobias Hartmann tobias.hartmann at oracle.com
Wed Jul 19 13:37:04 UTC 2017


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


More information about the valhalla-dev mailing list