Skip to content

Conversation

merykitty
Copy link
Member

@merykitty merykitty commented Jul 28, 2025

Hi,

Flat accesses prevent scalar replacement because they are mismatched accesses. It is also generally not possible to look through them, because the payload may contain an oop in the form of raw bits. As a result, this PR adds LoadFlatNode and StoreFlatNode, which act as high-level abstractions for atomic accesses on flat fields. When it is determined that there is no racing access on the flat field (e.g. because the holder object does not escape), these flat access nodes can be expanded into multiple accesses to each flattened fields, otherwise, they will be expanded into a sequence of inferring a payload and accessing memory with that payload.

I also fix an issue with deoptimization reallocation where we miss assigning the null marker of elements in a nullable flat array.

Please take a look and leave your reviews, thanks a lot.


Progress

  • Change must not contain extraneous whitespace

Issue

  • JDK-8364191: [lworld] Accesses to atomic flat fields prevent scalar replacement (Bug - P4)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/valhalla.git pull/1518/head:pull/1518
$ git checkout pull/1518

Update a local copy of the PR:
$ git checkout pull/1518
$ git pull https://git.openjdk.org/valhalla.git pull/1518/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 1518

View PR using the GUI difftool:
$ git pr show -t 1518

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/valhalla/pull/1518.diff

Using Webrev

Link to Webrev Comment

@bridgekeeper
Copy link

bridgekeeper bot commented Jul 28, 2025

👋 Welcome back qamai! A progress list of the required criteria for merging this PR into lworld will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

@openjdk
Copy link

openjdk bot commented Jul 28, 2025

@merykitty This change is no longer ready for integration - check the PR body for details.

@openjdk openjdk bot added ready Pull request is ready to be integrated rfr Pull request is ready for review labels Jul 28, 2025
@mlbridge
Copy link

mlbridge bot commented Jul 28, 2025

Webrevs

@TobiHartmann
Copy link
Member

Thanks for working on this @merykitty! I haven't reviewed this in yet but noticed a build failure:

[2025-08-04T17:23:31,905Z] /opt/mach5/mesos/work_dir/slaves/f7f8bd65-a387-4a2b-b519-702f2fefaf87-S170287/frameworks/1735e8a2-a1db-478c-8104-60c8b0af87dd-0196/executors/8d44f2d4-5a02-4ac8-95c1-3a37a1d82705/runs/1b653695-e82f-4430-a41f-d099d041486e/workspace/open/src/hotspot/share/opto/escape.cpp: In member function 'void ConnectionGraph::add_final_edges(Node*)':
[2025-08-04T17:23:31,905Z] /opt/mach5/mesos/work_dir/slaves/f7f8bd65-a387-4a2b-b519-702f2fefaf87-S170287/frameworks/1735e8a2-a1db-478c-8104-60c8b0af87dd-0196/executors/8d44f2d4-5a02-4ac8-95c1-3a37a1d82705/runs/1b653695-e82f-4430-a41f-d099d041486e/workspace/open/src/hotspot/share/opto/escape.cpp:1874:25: error: no matching function for call to 'ConnectionGraph::set_escape_state(PointsToNode*&, PointsToNode::EscapeState, const char [24])'
[2025-08-04T17:23:31,905Z]  1874 |         set_escape_state(field_value_ptn, PointsToNode::GlobalEscape, "store into a flat field");
[2025-08-04T17:23:31,905Z]       |         ~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[2025-08-04T17:23:31,905Z] In file included from /opt/mach5/mesos/work_dir/slaves/f7f8bd65-a387-4a2b-b519-702f2fefaf87-S170287/frameworks/1735e8a2-a1db-478c-8104-60c8b0af87dd-0196/executors/8d44f2d4-5a02-4ac8-95c1-3a37a1d82705/runs/1b653695-e82f-4430-a41f-d099d041486e/workspace/open/src/hotspot/share/opto/escape.cpp:38:
[2025-08-04T17:23:31,905Z] /opt/mach5/mesos/work_dir/slaves/f7f8bd65-a387-4a2b-b519-702f2fefaf87-S170287/frameworks/1735e8a2-a1db-478c-8104-60c8b0af87dd-0196/executors/8d44f2d4-5a02-4ac8-95c1-3a37a1d82705/runs/1b653695-e82f-4430-a41f-d099d041486e/workspace/open/src/hotspot/share/opto/escape.hpp:434:8: note: candidate: 'void ConnectionGraph::set_escape_state(PointsToNode*, PointsToNode::EscapeState)'
[2025-08-04T17:23:31,905Z]   434 |   void set_escape_state(PointsToNode* ptn, PointsToNode::EscapeState esc
[2025-08-04T17:23:31,905Z]       |        ^~~~~~~~~~~~~~~~
[2025-08-04T17:23:31,905Z] /opt/mach5/mesos/work_dir/slaves/f7f8bd65-a387-4a2b-b519-702f2fefaf87-S170287/frameworks/1735e8a2-a1db-478c-8104-60c8b0af87dd-0196/executors/8d44f2d4-5a02-4ac8-95c1-3a37a1d82705/runs/1b653695-e82f-4430-a41f-d099d041486e/workspace/open/src/hotspot/share/opto/escape.hpp:434:8: note:   candidate expects 2 arguments, 3 provided
[2025-08-04T17:23:32,241Z] lib/CompileJvm.gmk:172: recipe for target '/opt/mach5/mesos/work_dir/slaves/f7f8bd65-a387-4a2b-b519-702f2fefaf87-S170287/frameworks/1735e8a2-a1db-478c-8104-60c8b0af87dd-0196/executors/8d44f2d4-5a02-4ac8-95c1-3a37a1d82705/runs/1b653695-e82f-4430-a41f-d099d041486e/workspace/build/linux-x64/hotspot/variant-server/libjvm/objs/static/escape.o' failed
[2025-08-04T17:23:32,241Z] make[3]: *** [/opt/mach5/mesos/work_dir/slaves/f7f8bd65-a387-4a2b-b519-702f2fefaf87-S170287/frameworks/1735e8a2-a1db-478c-8104-60c8b0af87dd-0196/executors/8d44f2d4-5a02-4ac8-95c1-3a37a1d82705/runs/1b653695-e82f-4430-a41f-d099d041486e/workspace/build/linux-x64/hotspot/variant-server/libjvm/objs/static/escape.o] Error 1

@merykitty
Copy link
Member Author

@TobiHartmann Ah dummy mistake, I missed the NOT_PRODUCT guard, fixed it now.

@merykitty merykitty marked this pull request as draft August 18, 2025 17:32
@openjdk openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Aug 18, 2025
@TobiHartmann
Copy link
Member

Hi @merykitty Sorry for the delay in reviewing this, I was busy with the the array metadata refactoring and hunting down nasty AArch64 crashes (JDK-8364579 / JDK-8365996). This is great work! I see that you converted the PR back to draft, is it still ready to review?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants