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

[David Simms]

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
>>