RFR (M): Generate checks for vbox/vunbox; extend CI to include VCC-DVT mapping

Tobias Hartmann tobias.hartmann at oracle.com
Mon Apr 3 13:12:01 UTC 2017


Hi Zoltan,

On 03.04.2017 11:16, Zoltán Majó wrote:
>> - In graphKit.cpp, why do you set 'treat_throw_as_hot' to always true? Shouldn't the checks in line 549 be sufficient to set it (also for the vbox/vunbox bytecodes)?
> 
> I was curious how the code with pre-generated looks like. But you are right, we can use profiling information about traps and use pre-generated exceptions only if we hit the trap limit (just as in the "standard" VM).

Okay, thanks for checking!

>> - Since we don't support subtyping for value types, shouldn't we check for type equality instead of a subtype relation?
>>    - In vbox(), you can get the exact type of the ValueTypeNode via ValueTypeNode::value_klass() and you can check that for equality with the target_dvt_klass
> 
> I agree -- let's do that.

Looks good. I just wonder if we really need 'guarantee' for the static checks. Existing code uses 'assert' for similar checks but it doesn't matter too much for now.

>>    - In vunbox(), I would check for equality of the source_type and the target_vcc_klass
> 
> In some cases (e.g., when the declared type source_type is java.lang.Object) the check you mention is not sufficient. We need to generate a dynamic type check in that case -- test_64 covers that scenario.

Sure but what I meant is a dynamic check for type equality of the VCC (derived from the target value type) and the source type instead of a subtype check. Because we don't support subtyping, right? Test64 should only ever pass if 'vcc' is ValueCapableClass1.

For example, with VCC A and class B which is a subtype of A, vunbox of B to A$Value should fail, right? I think we should also add a test for this case.

Thanks,
Tobias


More information about the valhalla-dev mailing list