RFR (M): Add missing compiler checks
david.simms at oracle.com
Wed Jun 14 11:53:42 UTC 2017
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 !
On 14/06/17 12:55, Zoltán Majó wrote:
> please review the following change:
> 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
> 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,
More information about the valhalla-dev