Return value type fields in registers

[Maurizio Cimadamore]

Did you add the flag

-Dvalhalla.enableValueLambdaForms=true

to the mix? If not, the LF specialization will not kick in.

Maurizio


On 02/06/17 14:09, Tobias Hartmann wrote:
> Hi Roland,
>
> I've executed some quick testing on JPRT on top of Maurizio's latest patch:
> http://cr.openjdk.java.net/~mcimadamore/specialized_lforms_hs/
> http://cr.openjdk.java.net/~mcimadamore/specialized_lforms_jdk/
>
> I see multiple failures:
>
> ValueTypesTestBench fails with:
>
> #  Internal Error (/scratch/opt/jprt/T/P1/112707.tohartma/s/hotspot/src/cpu/x86/vm/macroAssembler_x86.cpp:889), pid=3999, tid=4000
> #  assert(false) failed: DEBUG MESSAGE: illegal bytecode sequence - method not verified
>
> Stack: [0x00007f9a958b1000,0x00007f9a959b2000],  sp=0x00007f9a959afcf0,  free space=1019k
> Native frames: (J=compiled Java code, A=aot compiled Java code, j=interpreted, Vv=VM code, C=native code)
> V  [libjvm.so+0x169ce1f]  VMError::report_and_die(int, char const*, char const*, __va_list_tag*, Thread*, unsigned char*, void*, void*, char const*, int, unsigned long)+0x15f
> V  [libjvm.so+0x169dbcf]  VMError::report_and_die(Thread*, char const*, int, char const*, char const*, __va_list_tag*)+0x2f
> V  [libjvm.so+0xacf6fd]  report_vm_error(char const*, int, char const*, char const*, ...)+0xdd
> V  [libjvm.so+0x11641c4]  MacroAssembler::debug64(char*, long, long*)+0x204
> j  compiler.valhalla.valuetypes.ValueTypeTestBench.test75()Qcompiler/valhalla/valuetypes/MyValue3;+4
> j  compiler.valhalla.valuetypes.ValueTypeTestBench.test75_verifier(Z)V+32
> v  ~StubRoutines::call_stub
>
> On Windows it still fails with
> Caused by: java.lang.RuntimeException: assertEquals: expected 1309945560682 to equal -2494
> 	at jdk.test.lib.Asserts.fail(Asserts.java:594)
> 	at jdk.test.lib.Asserts.assertEquals(Asserts.java:205)
> 	at jdk.test.lib.Asserts.assertEquals(Asserts.java:189)
> 	at jdk.test.lib.Asserts.assertEQ(Asserts.java:166)
> 	at compiler.valhalla.valuetypes.ValueTypeTestBench.test54_verifier(ValueTypeTestBench.java:1612)
>
> runtime/valhalla/valuetypes/VboxUnbox.java fails with
> java.lang.RuntimeException: assertTrue: expected true, was false
> 	at jdk.test.lib.Asserts.fail(Asserts.java:594)
> 	at jdk.test.lib.Asserts.assertTrue(Asserts.java:486)
> 	at jdk.test.lib.Asserts.assertTrue(Asserts.java:472)
> 	at runtime.valhalla.valuetypes.VboxUnbox.testCorrectBoxing(VboxUnbox.java:63)
> 	at runtime.valhalla.valuetypes.VboxUnbox.main(VboxUnbox.java:41)
>
>
> I'm not sure though if those problems are related to your fix. I'll re-run without your changes.
>
> Best regards,
> Tobias
>
> On 01.06.2017 17:51, Roland Westrelin wrote:
>> Thanks for the detailed review. Here is a new webrev:
>>
>> http://cr.openjdk.java.net/~roland/valhalla/returnconvention/webrev.02/
>>
>>> I'm seeing build failures [1] on JPRT and the ValueTypeTestBench fails
>>> on Windows [2] and crashes with an assert [3] on my machine.
>> Build failures should be fixed now. I made sure it builds with
>> precompiled headers disabled.
>>
>> I tried to figure out what could cause the windows failure but I can't
>> say I have a clue. Registers for returns should be the same but used in
>> a different order. I don't see why that would make a difference. I don't
>> have access to a windows machine so I don't see what I can do about it.
>>
>> The assert failure should be fixed now. I added a new test case to
>> reproduce it (test76). Interestingly it causes a crash with
>> ValueTypeReturnedAsFields off. I found that's because we allocate in
>> vreturn, we can actually throw in a return which is not something c2
>> expects.
>>
>> 2308   set_bci(InvocationEntryBci);
>>
>> in parse1.cpp breaks the exception handling logic. I commented it out
>> and everything seems to work. I also look at the code and saw no reason
>> for clearing the bci.
>>
>>> compile.cpp
>>> - Why is the change in line 2869 necessary?
>> It's not. Removed.
>>
>>> - It would be good to add a reference field to MyValue3 to extend
>>> coverage
>> Done. It caused some new failures in escape analysis so there's some new
>> changes there.
>>
>> Roland.
>>