<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=Windows-1252">
</head>
<body>
<div dir="auto">Hi Xiaohong, Fred,</div>
<div dir="auto"><br>
</div>
<div dir="auto"><br>
</div>
<div dir="auto"><span style="color:rgb(0, 0, 0);font-size:14.6667px;background-color:rgb(255, 255, 255);display:inline !important">> One is removing the followed "field" access check, which means the fields can be flattened no matter whether the field is "final"
 or not.</span><br>
</div>
<div dir="auto"><span style="color:rgb(0, 0, 0);font-size:14.6667px;background-color:rgb(255, 255, 255);display:inline !important"><br>
</span></div>
<div dir="auto">On final field flatting, it's good to always flatten them to enhance constant folding opportunities for C2 compiler. We may have an InlineTypeNodes with high indegree, but a
<i>getfield</i> will always get hold of constant value.</div>
<div dir="auto"><br>
</div>
<div dir="auto">For a field to be a true constant should we not check for both static and final accesses?</div>
<div dir="auto"><br>
</div>
<div dir="auto">Alternatively, since a primitive/value class objects are immutable with implicit final fields unless object in larval state so we may also relax final access check, WDYT ?</div>
<div dir="auto"><br>
</div>
<div dir="auto">Best Regards,</div>
<div dir="auto">Jatin </div>
<div dir="auto"><br>
</div>
<div id="mail-editor-reference-message-container" dir="auto"><br>
<hr style="display:inline-block;width:98%" tabindex="-1">
<div id="divRplyFwdMsg" style="font-size: 11pt;"><strong>From:</strong> valhalla-dev <valhalla-dev-retn@openjdk.org> on behalf of Frederic Parain <frederic.parain@oracle.com><br>
<strong>Sent:</strong> Thursday, July 6, 2023 6:28:16 PM<br>
<strong>To:</strong> Xiaohong Gong <Xiaohong.Gong@arm.com>; valhalla-dev@openjdk.org <valhalla-dev@openjdk.org><br>
<strong>Cc:</strong> nd <nd@arm.com><br>
<strong>Subject:</strong> Re: [External] : RE: Question on the inline type flattening decision<br>
</div>
<br>
<meta name="Generator" content="Microsoft Exchange Server">
<!-- converted from text --><font size="2"><span style="font-size:11pt;">
<div class="PlainText">Hi Xiaohong,<br>
<br>
<br>
I would recommend to modify the test driving the flattening decisions <br>
the following way:<br>
<br>
if (InlineFieldMaxFlatSize != 0<br>
<br>
      && (!(too_big_to_flatten | too_atomic_to_flatten | <br>
too_volatile_to_flatten) || fieldinfo.access_flags().is_final()) {<br>
<br>
<br>
By adding the first term "InlineFieldMaxFlatSize != 0 ", it will be <br>
obvious that this is the way to completely disable field flattening.<br>
<br>
The second term would remain the more specific tests on the field's <br>
properties to decide to flatten or not when flattening is enabled.<br>
<br>
<br>
What do you think of this proposal?<br>
<br>
<br>
Best Regards,<br>
<br>
<br>
Fred<br>
<br>
<br>
On 7/2/23 9:23 PM, Xiaohong Gong wrote:<br>
> Hi Frederic,<br>
><br>
> Thanks for looking at this issue. Yes, I was planning to fix this. But I'm not sure how to handle the relationship with "final" fields.<br>
><br>
> One is removing the followed "field" access check, which means the fields can be flattened no matter whether the field is "final" or not.<br>
><br>
> Another is changing the followed "||" to "&&", so that only the "final" fields that match the preconditions can be flattened.<br>
><br>
> So which one do you think is the defined behavior? The first one sounds reasonable to me. WDYT? Thanks a lot!<br>
><br>
> BTW, I'v filed this issue to: <a href="https://bugs.openjdk.org/browse/JDK-8311219">
https://bugs.openjdk.org/browse/JDK-8311219</a><br>
><br>
> Best Regards,<br>
> Xiaohong<br>
><br>
> -----Original Message-----<br>
> From: Frederic Parain <frederic.parain@oracle.com><br>
> Sent: Friday, June 30, 2023 9:28 PM<br>
> To: Xiaohong Gong <Xiaohong.Gong@arm.com>; valhalla-dev@openjdk.org<br>
> Cc: nd <nd@arm.com><br>
> Subject: Re: Question on the inline type flattening decision<br>
><br>
> Hi Xiaohong,<br>
><br>
><br>
> Thank you for reporting this. It looks like a bug to me, specifying<br>
> -XX:InlineFieldMaxFlatSize=0 should disable all field flattening, including final fields.<br>
><br>
> Do you want to fill a bug report or do you want me to take care of it?<br>
><br>
><br>
> Best Regards,<br>
><br>
><br>
> Fred<br>
><br>
><br>
> On 6/30/23 3:04 AM, Xiaohong Gong wrote:<br>
>> Hi,<br>
>><br>
>> I guess this is the right place to ask this question related to the<br>
>> flattening decision on inline type fields?<br>
>><br>
>> I met an issue when I was running the tests under<br>
>> “hotspot/jtreg/compiler/valhalla/inlinetypes” with<br>
>> “-XX:InlineFieldMaxFlatSize=0”. The intention is doing some testing by<br>
>> forcing the inline type fields not be flattened.  And I debugged some<br>
>> C2 code in `inlinetypenode.cpp` like `InlineTypeNode::load()`.  But I<br>
>> found that the code path has no difference, which means this flag<br>
>> cannot work and the field is flattened as without this flag.<br>
>><br>
>> And then, I checked the relative code<br>
>> <a href="https://urldefense.com/v3/__https://github.com/openjdk/valhalla/blob/lworld/src/hotspot/share/clas__;!!ACWV5N9M2RV99hQ!J7daZI_Hay5kWckchOY_VRTIwQvkJzF8AtUA0SEbDccLvtWEIcMKpkQY4IFKruqGd1K0_qzZXVvoPrHk3ys7z-iq8Q$">
https://urldefense.com/v3/__https://github.com/openjdk/valhalla/blob/lworld/src/hotspot/share/clas__;!!ACWV5N9M2RV99hQ!J7daZI_Hay5kWckchOY_VRTIwQvkJzF8AtUA0SEbDccLvtWEIcMKpkQY4IFKruqGd1K0_qzZXVvoPrHk3ys7z-iq8Q$</a><br>
>> sfile/fieldLayoutBuilder.cpp#L759 to find out the reason. It checks<br>
>> the three necessary conditions (i.e.<br>
>> “the code size comparison with `InlineFieldMaxSize`”, “atomic” or<br>
>> “volatile” fields) firstly, which is the right behavior I think. But<br>
>> then the result is “||” with the `final` access flag. Which means if<br>
>> the field is `final`, it will be flattened anyway regardless of the<br>
>> necessary three limitations.<br>
>><br>
>> So is this the expected behavior? Does this mean the inline type field<br>
>> can always be flattened if it is declared with `final`? But I didn’t<br>
>> find any descriptions on the flattening decision related to the<br>
>> `final` flag.<br>
>><br>
>> Many thanks if any help on this!<br>
>><br>
>> Best Regards,<br>
>><br>
>> Xiaohong<br>
>><br>
</div>
</span></font><br>
</div>
</body>
</html>