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

[Zoltán Majó]

Hi Tobias,


thank you for the feedback. Please see below my replies to the issues 
you've raised.

On 03/30/2017 10:26 AM, Tobias Hartmann wrote:
> Hi Zoltan,
>
> On 28.03.2017 13:20, Zoltán Majó wrote:
>> http://cr.openjdk.java.net/~zmajo/valhalla/03.vbox-vunbox-checks/webrev.00/
>>
>> The patch adds checks to vbox/vunbox that are missing in the current implementation. The patch enables C2 to generate code that includes null checks and type checks (where required). Other checks (e.g., checking if the source/target class of the bytecode instruction is loaded and if it is a value-capable-class/derived value type) are performed at compile-time. The patch also extends the CI to mirror the VCC-DVT mapping to the compiler.
> Here are some comments/questions:
> - I think it's sufficient to have the "// TODO: Check if updating flag works properly" in one place

OK, I changed to code to have the comment only at one place.

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

> - 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.

>    - 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.

Here is the updated webrev:
http://cr.openjdk.java.net/~zmajo/valhalla/03.vbox-vunbox-checks/webrev.01/

I ran all hotspot_valhalla tests, all pass. If the changes look good, I 
will run the patch through JPRT and with Octane before pushing.

Thank you!

Best regards,


Zoltan

>
> Thanks,
> Tobias