RFR(L): More migration to the L-world

John Rose john.r.rose at oracle.com
Sun Mar 11 04:59:36 UTC 2018

This has been enjoyable background reading for a few days.

It all looks good, but I noticed something that will need
changing at some point:

+    __ jmp(notVolatile); // value types are never volatile

Value types will become volatile at some point, so the
comment is slightly misleading.  Suggest changing
it to jmp(atosDone) with a suitable label definition.

And/or, the comment could be "currently we do not flatten
volatile value types".

Your code will be correct, I think, even when we *do*
support value-type volatiles, since our first implementation
of volatiles will surely be heap-buffering.  The gating
logic in classFileParser that decides when to flatten
a field (currently turned off) should notice if a field
is volatile and back off to heap buffering, just as if
the field type itself were jumbo sized.

(We could experiment with keeping volatiles flat if
they are a machine word or smaller, but that's not
likely to pay off.  Eventually I hope we will support
flattened volatiles larger than a machine word (or
pair of words) using seq-locks.  It will be a while
before we are ready for that experiment; too many
more urgent things to do.)

Here's a sketch of how to support volatility of fields
and atomicity of types:

-+      if ((ValueFieldMaxFlatSize < 0) || (vk->size_helper() * HeapWordSize) <= ValueFieldMaxFlatSize) {
++     if (((ValueFieldMaxFlatSize < 0) || (vk->size_helper() * HeapWordSize) <= ValueFieldMaxFlatSize)
++          && !fs.access_flags().is_volatile() && !vk->access_flags().is_volatile()) {

Here's a nit in the nearby code:

       assert(klass->access_flags().is_value_type(), "Value type expected");

This should actually be a call to check_property,
and throw CFE if the predicate fails.

I see that the templateTable code doesn't write back a default
value to a null container, and the runtime C code doesn't either.
Shouldn't the C code take care of that, so the interpreter doesn't
always call into the runtime?  It would also make things easier
for the JIT, at least for statics

Given the new special roles of nulls, we should consider putting
TypeEntries::null_seen bits in the profiles for the field access
bytecodes.  Currently we don't put profile records on those.
(Null-seen on a getfield or putfield won't refer to the receiver
but to the value being loaded or stored.  Maybe that's a
different bit?)

Keep up the great work, Fred!

— John

On Mar 5, 2018, at 8:12 AM, Frederic Parain <frederic.parain at oracle.com> wrote:
> Please review the following patch:
> http://cr.openjdk.java.net/~fparain/field_flattening/webrev.00/index.html
> This patch was started as a fix for field flattening but it ended
> being an omnibus patch addressing the following issues:
>  - Fixed value types runtime tests accordingly to the new language
>    support in javac
>  - Fixed storage and propagation of the ACC_FLATTENABLE flag in JVM
>    meta-data
>  - Fixed value flattening for value fields with the ACC_FLATTENABLE
>    flag set
>  - Fixed uninitialized value for fields with ACC_FLATTENABLE flag set
>  - Fixed klass initialization sequence for ACC_FLATTENABLE support
>  - Implemented flattenable semantic for value fields with
>    ACC_FLATTENABLE flag set
>  - Added test to verify flattenable semantic
>  - Fixed a bug in the verifier (withfield)
> With this patch, most tests in the runtime/valhalla/valuetypes
> directory now pass in interpreted mode. Only 3 tests still fail,
> 2 because of the value array code that has not been migrated to
> the L-world, 1 because of the Bytecode API which has not been
> migrated either.
> Builds and tests (TEST=hotspot_valhalla_runtime) have been run on
> Linux/X64 and MacOSX/X64.
> Thank you,
> Fred

More information about the valhalla-dev mailing list