RFR: JDK-8287061: Support for rematerializing scalar replaced objects participating in allocation merges [v14]

Vladimir Ivanov vlivanov at openjdk.org
Mon Jun 5 20:31:03 UTC 2023


On Mon, 5 Jun 2023 20:27:42 GMT, Vladimir Ivanov <vlivanov at openjdk.org> wrote:

>>> Why is it limited to non-product builds? It's valuable irrespective of build flavor.
>> 
>> This is because `print_on` in `AnyObj` is only defined in non-product builds. I based implementation of `ObjectMergeValue::print_on` on `ObjectValue::print_on`. In `ObjectValue::print_on` fields aren't printed in product builds.
>> 
>>> Any particular reason for that? You could add is_object_merge case in ObjectValue::print_oninstead and extendObjectValue::print_fields_onto coverObjectMergeValue case.
>> 
>> I'll do that then.
>> 
>>> Also, formatting is broken.
>> 
>> Can you please share an example? If you mean the tabs on lines 303/304/306/307 I added those because I thought would make the code easier to read, but if you want I can definitely remove that.
>
>>  If you mean the tabs on lines 303/304/306/307
> 
> Yes, it confused me. As an alternative, you could put selector and merge_pointer-related statements on the same line, but I'm not sure how much it improves readability:
> 
>   st->print(", selector=""); _selector->print_on(st); st->print(""");

A couple of suggestions about the output:
* `merge`: it's clearer to call it `merge_obj`
* `obj` vs `merge` output: obj output is duplicated in ScopeDesc entries and Objects sections; before it was a short version printed in Locals/Expressions and all the details were included in Objects; I like to see field locations in the short version, but including everything looks way too much IMO;
* it makes sense to include selector and merge_pointer info in short version, but `is_root` can be omitted

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/12897#discussion_r1218558295


More information about the hotspot-compiler-dev mailing list