Question on the inline type flattening decision

[Xiaohong Gong]

Hi Frederic,

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. 

One is removing the followed "field" access check, which means the fields can be flattened no matter whether the field is "final" or not.

Another is changing the followed "||" to "&&", so that only the "final" fields that match the preconditions can be flattened.

So which one do you think is the defined behavior? The first one sounds reasonable to me. WDYT? Thanks a lot! 

BTW, I'v filed this issue to: https://bugs.openjdk.org/browse/JDK-8311219

Best Regards,
Xiaohong

-----Original Message-----
From: Frederic Parain <frederic.parain at oracle.com> 
Sent: Friday, June 30, 2023 9:28 PM
To: Xiaohong Gong <Xiaohong.Gong at arm.com>; valhalla-dev at openjdk.org
Cc: nd <nd at arm.com>
Subject: Re: Question on the inline type flattening decision

Hi Xiaohong,


Thank you for reporting this. It looks like a bug to me, specifying
-XX:InlineFieldMaxFlatSize=0 should disable all field flattening, including final fields.

Do you want to fill a bug report or do you want me to take care of it?


Best Regards,


Fred


On 6/30/23 3:04 AM, Xiaohong Gong wrote:
>
> Hi,
>
> I guess this is the right place to ask this question related to the 
> flattening decision on inline type fields?
>
> I met an issue when I was running the tests under 
> “hotspot/jtreg/compiler/valhalla/inlinetypes” with 
> “-XX:InlineFieldMaxFlatSize=0”. The intention is doing some testing by 
> forcing the inline type fields not be flattened.  And I debugged some
> C2 code in `inlinetypenode.cpp` like `InlineTypeNode::load()`.  But I 
> found that the code path has no difference, which means this flag 
> cannot work and the field is flattened as without this flag.
>
> And then, I checked the relative code
> https://github.com/openjdk/valhalla/blob/lworld/src/hotspot/share/clas
> sfile/fieldLayoutBuilder.cpp#L759 to find out the reason. It checks 
> the three necessary conditions (i.e.
> “the code size comparison with `InlineFieldMaxSize`”, “atomic” or 
> “volatile” fields) firstly, which is the right behavior I think. But 
> then the result is “||” with the `final` access flag. Which means if 
> the field is `final`, it will be flattened anyway regardless of the 
> necessary three limitations.
>
> So is this the expected behavior? Does this mean the inline type field 
> can always be flattened if it is declared with `final`? But I didn’t 
> find any descriptions on the flattening decision related to the 
> `final` flag.
>
> Many thanks if any help on this!
>
> Best Regards,
>
> Xiaohong
>