[9] RFR (M): VM should constant fold Unsafe.get*() loads from final fields

[Vladimir Ivanov]

Thanks, Paul!

> I like the test, you have almost hand rolled your own specializer :-)
I tried hard to avoid bytecode generation, but having 8 almost identical 
copies of the same logic is too much :-)

> A minor point. Since you have created a ClassWriter with "ClassWriter.COMPUTE_MAXS | ClassWriter.COMPUTE_FRAMES" can you remove the "mv.visitMax(0, 0)" calls?
>
>
> I was a little confused by the code that checked the expected result against the actual result.
>
> I am guessing the white box methods return -1 if the value is not a constant and 1 if it is. (Perhaps that can be documented, if even those methods may eventually reside somewhere else.) Whereas, Generator.expected returns 0 or 1.
>
>   118         if (direct != unsafe || // difference between testDirect & testUnsafe
>   119             (unsafe != -1 && expected != unsafe)) // differs from expected, but ignore "unknown"(-1) result
>   120         {
>   121             throw new AssertionError(String.format("%s: e=%d; d=%d; u=%d", t.name(), expected, direct, unsafe));
>   122         }
>
> I don't quite understand why  "unknown"(-1)  can be ignored.
>
> Can that be changed to the following if Generator.expected returned the same values as the WB methods?
>
>    if (direct != unsafe || unsafe != expected) { ... }
>
> ?
Though I've removed WB.isCompileConstat() in the updated version, I'll 
elaborate on that. WB.isCompileConstat() returns 3 values:
   * 0: the argument is not a compile-time constant;
   * 1: the argument is a compile-time constant;
   * -1: it's not known whether it is constant or not

"-1" signals that there's no data from JIT. WB.isCompileConstat() is 
intrinsified by a JIT-compiler. I implemented the intrinsics only in C2, 
so if you run the test in -Xint mode or with Client VM, 
WB.isCompileConstant() will always return -1.

In order to be resilient, the test ignores "-1" when it checks the 
results.

Best regards,
Vladimir Ivanov
>
> Paul.
>
> On Jun 17, 2015, at 6:38 PM, Vladimir Ivanov <vladimir.x.ivanov at oracle.com> wrote:
>
>> http://cr.openjdk.java.net/~vlivanov/8078629/webrev.00/
>> https://bugs.openjdk.java.net/browse/JDK-8078629
>>
>> Direct(getfield/getstatic) read operations are faster than unsafe reads from constant Java fields, since VM doesn't constant fold unsafe loads. Though VM tries hard to recover field metadata from its offset, it doesn't optimize unsafe ones even if it has all necessary info in its hands.
>>
>> The fix is to align the behavior and share relevant code between C2 parser and intrinsic expansion logic.
>>
>> For testing purposes, I extended whitebox API to check whether a value is a compile-time constant. The whitebox test enumerates all combinations of a field and ensures that the behavior is consistent between bytecode and unsafe reads.
>>
>> Testing: focused whitebox tests, hotspot/test/compiler, jdk/test/java/lang/invoke, octane (for performance measurements)
>>
>> Thanks!
>>
>> Best regards,
>> Vladimir Ivanov
>