RFR (M): Add missing compiler checks

[David Simms]

The changes to "src/share/vm/oops/*" and 
"src/share/vm/interpreter/interpreterRuntime.cpp" look good.

One nit-pick is the use of C cast to "ValueKlass*" rather than 
"ValueKlass::cast()" like we generally do for Klass casting.

The extra field in TypeArrayKlass is minimal, given the fixed number of 
types (even with multi-dim).

Thanks for the extra klass check in vastore, btw !

Cheers
/David Simms



On 14/06/17 12:55, Zoltán Majó wrote:
> Hi,
>
>
> please review the following change:
> http://cr.openjdk.java.net/~zmajo/valhalla/04.checks/webrev.00/
>
> The change adds some (or hopefully all) checks that C2 currently does 
> not generate but will most likely be required by the MVT 
> specification. I tested the change with JPRT (x86_64), no failures 
> have appeared. I'm currently running the hotspot/compiler JTREG tests 
> locally.
>
> A few notes about the change:
>
> - Moving _element_klass from ObjArrayKlass to ArrayKlass is necessary 
> because that way the klass of the array elements is found at the same 
> offset in both ObjArrayKlass and ValueArrayKlass instances (and we 
> need the offset to implement type checks). However, _element_klass is 
> now present in TypeArrayKlass as well (where it is not needed). Maybe 
> a better way to organize the class hierarchy would be to put 
> _element_klass into a new common subclass X of [Obj|Value]ArrayClass; 
> both X and TypeArrayKlass would then be direct subclasses of 
> ArrayKlass. Maybe it's better to deal with this aspect once the we're 
> closer to the final specification/design of MVT.
>
> - The change adds a type check to 
> InterpreterRuntime::value_array_store. I assumed (and expect) vastore 
> will behave similarly to aastore and requires the element type of 
> src/dst to match. Please let me know if that is an incorrect assumption.
>
> - The bytecodes vdefault/vwithfield can throw an OutOfMemoryError if 
> no space is available for allocation. With the current C2 
> implementation allocations can be delayed to a later point in the 
> program (e.g., when a deoptimization happens). If no free memory is 
> available at that point to allocate memory for a value type, the OOME 
> will appear later than the vdefault/vwithfield bytecode that actually 
> caused it. Tobias and I discussed about this aspect and it's likely 
> that this is not a problem as re-allocation of scalar-replaced objects 
> is similar and is also performed at deoptimization. However, more 
> investigation is needed to confirm that.
>
> Thank you!
>
> Best regards,
>
>
> Zoltan
>