RFR 8227494 [lworld][c1] Optimize withfield bytecode into putfield

[Tobias Hartmann]

Hi Ioi,

okay, thanks for the explanation.

Best regards,
Tobias

On 25.07.19 22:32, Ioi Lam wrote:
> Hi Tobias, thanks for the review.
> 
> 
> On 7/25/2019 1:59 AM, Tobias Hartmann wrote:
>> Hi Ioi,
>>
>> this looks good to me!
>>
>> Why is the change in c1_GraphBuilder.cpp:1879 necessary?
> 
> I have modified the line to the following:
> 
>   if (!holder->is_loaded()
>       || needs_patching /* FIXME: 8228634 - field_modify->will_link() may incorrectly return false */
>       ) {
> 
> I'll add more descriptions into the bug, but so far it seems to affect only with __Withfield()
> macros that operates on the final fields of other classes. It doesn't affect "normal" Java code that
> initializes the fields of an inline class in its own <init> method.
> 
> Thanks
> - Ioi
> 
>> Best regards,
>> Tobias
>>
>> On 24.07.19 07:30, Ioi Lam wrote:
>>> https://bugs.openjdk.java.net/browse/JDK-8227494
>>> http://cr.openjdk.java.net/~iklam/valhalla/8227494-withfield-optimization.v01/
>>>
>>>
>>> This optimization significantly reduces the amount of code generated by C1 for
>>> withfield bytecodes.
>>>
>>> In the <init> methods of value classes, it's quite common to have a sequence like
>>> this, where we load a value object from a local slot, and overwrite the same local
>>> slot with a modified copy of the value object.
>>>
>>>       defaultvalue #1 // class compiler/valhalla/valuetypes/MyValue1
>>>       astore 9
>>>       ...
>>>       iload_0
>>>       aload 9
>>>       swap
>>>       withfield #7 // Field x:I
>>>       astore 9
>>>
>>> If this object was created by defaultvalue, and has not escaped, and is not stored
>>> in any other local slots, we can effectively treat the withfield/astore
>>> sequence as a single putfield bytecode.
>>>
>>> I implemented a very rudimentary escape analysis in c1_GraphBuilder.cpp. It seems like
>>> there's no need to follow back branches like this:
>>>
>>>          static FooValue test4() {
>>>              FooValue v = FooValue.default;
>>>              for (int i=1; i<=2; i++) {
>>>                  v = __WithField(v.x, i);
>>>                  v = __WithField(v.y, i);
>>>                  set_foo_static_if_null(v);
>>>              }
>>>
>>>              return v;
>>>          }
>>>
>>> The local slot for "v" in the loop body is a Phi node (not a NewValueTypeInstance node),
>>> so it will fail the "obj->is_optimizable_for_withfield()" check (which will only return true
>>> from NewValueTypeInstance) in GraphBuilder::withfield().
>>>
>>> Thanks
>>> - Ioi
>>>
>