RFR(M): optional flattening - interpreter/runtime part

[Frederic Parain]

Tobias, Mr Simms,

Thank you for the reviews, I’ve fixed all points reported by Tobias.

Fred


> On Sep 20, 2017, at 04:27, David Simms <david.simms at oracle.com> wrote:
> 
> 
> After Tobias's comments, looks good !
> 
> 
> On 20/09/17 09:16, Tobias Hartmann wrote:
>> Hi Fred,
>> 
>> On 19.09.2017 23:24, Frederic Parain wrote:
>>> http://cr.openjdk.java.net/~fparain/optional-flattening-push/webrev.02/index.html 
>> 
>> Your changes look good to me. I tested my patches for 8185556 and 8186716 on top and all tests pass.
>> 
>> Here are some minor suggestions:
>> 
>> classFileParser.cpp:
>> - line 3991: comment indentation is broken
>> 
>> interpreterRuntime.cpp
>> - line 279: missing whitespace after ","
>> - line 347 and 358: variable "res" is declared twice with different types
>> 
>> valueKlass.cpp
>> - line 317: there is an indentation problem in the if true branch
>> 
>> No need to send an updated webrev.
>> 
>> Thanks,
>> Tobias
>> 
>>>> On Sep 14, 2017, at 11:03, Tobias Hartmann <tobias.hartmann at oracle.com> wrote:
>>>> 
>>>> Hi Fred,
>>>> 
>>>> I've just verified that the problem is solved with your latest webrev!
>>>> 
>>>> Thanks,
>>>> Tobias
>>>> 
>>>> On 14.09.2017 16:50, Frederic Parain wrote:
>>>>> Tobias,
>>>>> Thank you for catching that.
>>>>> There’s a bug in the layout computation, optional flattening code moves non-flattened
>>>>> values from the value types section of the layout to the nonstatic_oop section, but the size
>>>>> of the later was not updated accordingly.
>>>>> Here’s an updated webrev fixing this issue:
>>>>> http://cr.openjdk.java.net/~fparain/optional-flattening-interp/webrev.01/index.html 
>>>>> Regards,
>>>>> Fred
>>>>>> On Sep 14, 2017, at 03:46, Tobias Hartmann <tobias.hartmann at oracle.com> wrote:
>>>>>> 
>>>>>> Hi Fred,
>>>>>> 
>>>>>> there seems to be a problem with the field layout:
>>>>>> 
>>>>>> compiler.valhalla.valuetypes.MyValue1: field layout
>>>>>>   @ 16 --- instance fields start ---
>>>>>>   @ 24 "x" I
>>>>>>   @ 16 "y" J
>>>>>>   @ 32 "z" S
>>>>>>   @ 40 "o" Ljava.lang.Integer;
>>>>>>   @ 48 "oa" [I
>>>>>>   @ 56 "v1" Qcompiler.valhalla.valuetypes.MyValue2;
>>>>>>   @ 64 "v2" Qcompiler.valhalla.valuetypes.MyValue2;
>>>>>>   @ 28 "c" I
>>>>>>   @ 56 --- instance fields end ---
>>>>>>   @ 56 --- instance ends ---
>>>>>>   @176 --- static fields start ---
>>>>>>   @192 "s" I
>>>>>>   @184 "sf" J
>>>>>>   @176 "v3" Qcompiler.valhalla.valuetypes.MyValue2;
>>>>>>   @200 --- static fields end ---
>>>>>> 
>>>>>>   OopMapBlocks:   1  /  4
>>>>>>     Offset:  40  - 64 Count:   4
>>>>>> 
>>>>>> "instance fields end" points to offset 56 which is not correct because the non-flattened field v2 is at offset 64.
>>>>>> 
>>>>>> Thanks,
>>>>>> Tobias
>>>>>> 
>>>>>> On 13.09.2017 21:41, Frederic Parain wrote:
>>>>>>> Greetings,
>>>>>>> Please review this changeset implementing optional field flattening
>>>>>>> in the interpreter and the runtime. The support for C2 is provided by
>>>>>>> Tobias in a separate changeset.
>>>>>>> http://cr.openjdk.java.net/~fparain/optional-flattening-interp/webrev.00/index.html 
>>>>>>> Note that trying to run some programs/tests with this changeset and
>>>>>>> not Tobias’ changeset is likely to cause crashes unless interpreted mode
>>>>>>> is forced and passing/returning values in registers is disabled.
>>>>>>> Thank you,
>>>>>>> Fred
>>> 
>