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