JDK-8258894 : C2: Forbid GCM to move stores into loops
  • Type: Bug
  • Component: hotspot
  • Sub-Component: compiler
  • Affected Version: 11,17
  • Priority: P3
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2020-12-23
  • Updated: 2025-02-12
  • Resolved: 2021-01-27
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 11 JDK 17
11.0.18-oracleFixed 17 b08Fixed
Related Reports
Blocks :  
Duplicate :  
Relates :  
Relates :  
Description
Currently, GCM considers it legal to move stores into inner loops, and relies on the frequency-based heuristic in PhaseCFG::hoist_to_cheaper_block() to prevents such movements from happening.

Moving a store into an inner loop should be however seen as illegal, as it breaks the invariant that, after GCM, memory definitions should not interfere. For example:

Original placement of store in B1 (loop L1):

B1 (L1):
  m1 <- ..
  m2 <- store m1, ..
B2 (L2):
  jump B2
B3 (L1):
  .. <- .. m2, ..

Wrong "hoisting" of store to B2 (in loop L2, child of L1):

B1 (L1):
  m1 <- ..
B2 (L2):
  m2 <- store m1, ..
  # Wrong: m1 and m2 interfere at this point.
  jump B2
B3 (L1):
  .. <- .. m2, ..

Currently, such illegal movements are prevented by pinning stores to their original block, if the CFG is irreducible; and by the properties of the frequency estimation heuristic, if the CFG is reducible. This RFE proposes discarding the illegal movements beforehand, so that decisions that affect correctness are not left to the heuristic's discretion. More specifically, stores and other memory-defining nodes should not be allowed to move into deeper loops than the ones they originally belong to.

The method testReducible() in test/hotspot/jtreg/compiler/codegen/TestGCMStorePlacement.java illustrates the issue: GCM considers moving counter++ into the loop, and only the frequency estimation heuristic prevents it from happening.
Comments
A pull request was submitted for review. URL: https://git.openjdk.org/jdk11u-dev/pull/1401 Date: 2022-10-07 19:53:02 +0000
07-10-2022

Fix request [11u] I backport this for parity with 11.0.18-oracle. A C2 change not really small, medium risk. I think we should backport it to stay on par. Two copyrights resolved and two chunks because there is #ifdef SPARC code in 11. Adapted the SPARC code, too. Test passes, but also passes without the fix.
07-10-2022

Thanks, Roberto!
29-08-2022

I just double-checked and I think this is an appropriate fix for JDK-8288975, see the explanation in https://bugs.openjdk.org/browse/JDK-8288975?focusedCommentId=14520016&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14520016 .
25-08-2022

[~thartmann] yes, will double check.
18-08-2022

ILW = Same as JDK-8288975 = P3
17-08-2022

Converting this to a bug because [~roland]'s analysis of JDK-8288975 showed that the issue is fixed by this patch. [~rcastanedalo] it would probably be good to double check that this is not a side effect that hides the issue.
17-08-2022

Changeset: f353fcf2 Author: Roberto CastaƱeda Lozano <rcastanedalo@openjdk.org> Committer: Tobias Hartmann <thartmann@openjdk.org> Date: 2021-01-27 15:08:39 +0000 URL: https://git.openjdk.java.net/jdk/commit/f353fcf2
27-01-2021