8302158: PPC: test/jdk/jdk/internal/vm/Continuation/Fuzz.java: AssertionError: res: false shouldPin: false #12557

reinrich · 2023-02-14T14:10:08Z

This fixes the linked issue by trimming the caller of a frame to be deoptimized back to its unextended_sp iff it is compiled. The creation of the section dead after deoptimization shown in the attachment yield_after_deopt_failure.log is prevented by this.

A new mode is added to the test BasicExt.java where all frames are deoptimized after a yield operation. The issue can be deterministically reproduced with the new mode. It's not worth to execute all test cases with the new mode though. Instead ContinuationCompiledFramesWithStackArgs_3c4 is always executed a 2nd time in this mode.

Before this BasicExt.java was refactored for better argument processing and representation of the test modes.
Also the try-catch-clause in the main method had to be changed to rethrow the caught exception because without this the test would have succeeded.

Testing: jtreg tests tier 1-4 on standard platforms and also on ppc64le.

Progress

Change must be properly reviewed (1 review required, with at least 1 Reviewer)
Change must not contain extraneous whitespace
Commit message must refer to an issue

Issue

JDK-8302158: PPC: test/jdk/jdk/internal/vm/Continuation/Fuzz.java: AssertionError: res: false shouldPin: false

Reviewers

Goetz Lindenmaier (@GoeLin - Reviewer) ⚠️ Review applies to 7ca17977
Martin Doerr (@TheRealMDoerr - Reviewer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 12557

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/12557.diff

bridgekeeper · 2023-02-14T14:11:21Z

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

openjdk · 2023-02-14T14:13:18Z

@reinrich The following labels will be automatically applied to this pull request:

core-libs
hotspot

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing lists. If you would like to change these labels, use the /label pull request command.

mlbridge · 2023-02-14T14:22:31Z

Webrevs

01: Full - Incremental (9d647388)
00: Full (7ca17977)

GoeLin

LGTM

GoeLin · 2023-02-15T12:35:52Z

src/hotspot/cpu/ppc/sharedRuntime_ppc.cpp

@@ -3090,6 +3090,11 @@ void SharedRuntime::generate_deopt_blob() {

  // stack: (caller_of_deoptee, ...).

+  // Freezing continuation frames requires that the caller is trimmed to unextended sp if compiled.
+  Register caller_sp = R23_tmp3;


Can the caller be interpreted? I.e., not compiled?
Then it might be helpful to comment like
// If not compiled this contains sp() resulting in a resize of 0.

Yes it can be interpreted. I've extended the comment for this case.

openjdk · 2023-02-15T12:37:42Z

@reinrich This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8302158: PPC: test/jdk/jdk/internal/vm/Continuation/Fuzz.java: AssertionError: res: false shouldPin: false

Reviewed-by: goetz, mdoerr

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 49 new commits pushed to the master branch:

1480d41: 8208470: Type annotations on inner type that is an array component
573c316: 8224980: FLAG_SET_ERGO silently ignores invalid values
3ba1560: 8302026: Port fdlibm inverse trig functions (asin, acos, atan) to Java
861e302: 8302163: Speed up various String comparison methods with ArraysSupport.mismatch
50dcc2a: 8301460: Clean up LambdaForm to reference BasicType enums directly
28f5250: 8302127: Remove unused arg in write_ref_field_post
0c96584: 8301225: Replace NULL with nullptr in share/gc/shenandoah/
26b111d: 8301700: Increase the default TLS Diffie-Hellman group size from 1024-bit to 2048-bit
5238817: 8301463: Code in DatagramSocket still refers to resolved JDK-8237352
11194e8: 8302325: Wrong comment in java.base/share/native/libjimage/imageFile.hpp
... and 39 more: https://git.openjdk.org/jdk/compare/0458d3825c0b6ba215a87143ad472acdcba59f40...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

TheRealMDoerr

LGTM. Thanks for fixing!

TheRealMDoerr · 2023-02-15T14:42:22Z

src/hotspot/cpu/ppc/frame_ppc.cpp

@@ -421,8 +421,12 @@ void frame::describe_pd(FrameValues& values, int frame_no) {
 #endif

 intptr_t *frame::initial_deoptimization_info() {
-  // unused... but returns fp() to minimize changes introduced by 7087445
-  return fp();
+  // `this` is the caller of the deoptee. We want to trimm it, if compiled, to


I believe "to trim" should contain only one 'm' in English.

TheRealMDoerr

Thanks!

reinrich · 2023-02-17T13:18:20Z

Thanks for the reviews!

/integrate

openjdk · 2023-02-17T13:20:16Z

Going to push as commit b8c9d6c.
Since your change was applied there have been 76 commits pushed to the master branch:

dc55a7f: 8302202: Incorrect desugaring of null-allowed nested patterns
c4ffe4b: 8301494: Replace NULL with nullptr in cpu/arm
4f1cffd: 8302674: Parallel: Remove unused methods in MutableNUMASpace
c91cd28: 8301481: Replace NULL with nullptr in os/windows
47ca577: 8301491: C2: java.lang.StringUTF16::indexOfChar intrinsic called with negative character argument
49eb68b: 8296158: Refactor the verification of CDS region checksum
655a712: 8301444: Port fdlibm hyperbolic transcendental functions to Java
b242eef: 8280419: Remove dead code related to VerifyThread and verify_thread()
4ce493f: 8302225: SunJCE Provider doesn't validate key sizes when using 'constrained' transforms for AES/KW and AES/KWP
a39cf2e: 8301753: AppendFile/WriteFile has differences between make 3.81 and 4+
... and 66 more: https://git.openjdk.org/jdk/compare/0458d3825c0b6ba215a87143ad472acdcba59f40...master

Your commit was automatically rebased without conflicts.

openjdk · 2023-02-17T13:20:28Z

@reinrich Pushed as commit b8c9d6c.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

reinrich added 4 commits February 13, 2023 22:25

BasicExt.java test: rethrow caught exception to indicate failure

a86d83c

BasicExt.java test: use enums for parameters

1effc64

Deoptimize after yield

fb3efe8

Trimm compiled caller of deoptee to unextended sp

7ca1797

reinrich changed the title ~~Ppc fuzz issues~~ 8302158: PPC: test/jdk/jdk/internal/vm/Continuation/Fuzz.java: AssertionError: res: false shouldPin: false Feb 14, 2023

openjdk bot added hotspot hotspot-dev@openjdk.org core-libs core-libs-dev@openjdk.org labels Feb 14, 2023

reinrich marked this pull request as ready for review February 14, 2023 14:16

openjdk bot added the rfr Pull request is ready for review label Feb 14, 2023

GoeLin approved these changes Feb 15, 2023

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Feb 15, 2023

TheRealMDoerr approved these changes Feb 15, 2023

View reviewed changes

reinrich added 3 commits February 15, 2023 20:16

Spelling

8e0d64f

Improve comment

0edbcb7

Improve comment

9d64738

TheRealMDoerr approved these changes Feb 16, 2023

View reviewed changes

openjdk bot added the integrated Pull request has been integrated label Feb 17, 2023

openjdk bot closed this Feb 17, 2023

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Feb 17, 2023

reinrich deleted the PPC_Fuzz_issues branch February 20, 2023 12:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

8302158: PPC: test/jdk/jdk/internal/vm/Continuation/Fuzz.java: AssertionError: res: false shouldPin: false #12557

8302158: PPC: test/jdk/jdk/internal/vm/Continuation/Fuzz.java: AssertionError: res: false shouldPin: false #12557

reinrich commented Feb 14, 2023 •

edited by openjdk bot

Loading

bridgekeeper bot commented Feb 14, 2023

openjdk bot commented Feb 14, 2023

mlbridge bot commented Feb 14, 2023 •

edited

Loading

GoeLin left a comment

GoeLin Feb 15, 2023

reinrich Feb 15, 2023

openjdk bot commented Feb 15, 2023 •

edited

Loading

TheRealMDoerr left a comment

TheRealMDoerr Feb 15, 2023

TheRealMDoerr left a comment

reinrich commented Feb 17, 2023

openjdk bot commented Feb 17, 2023

openjdk bot commented Feb 17, 2023

8302158: PPC: test/jdk/jdk/internal/vm/Continuation/Fuzz.java: AssertionError: res: false shouldPin: false #12557

8302158: PPC: test/jdk/jdk/internal/vm/Continuation/Fuzz.java: AssertionError: res: false shouldPin: false #12557

Conversation

reinrich commented Feb 14, 2023 • edited by openjdk bot Loading

Progress

Issue

Reviewers

Reviewing

bridgekeeper bot commented Feb 14, 2023

openjdk bot commented Feb 14, 2023

mlbridge bot commented Feb 14, 2023 • edited Loading

Webrevs

GoeLin left a comment

Choose a reason for hiding this comment

GoeLin Feb 15, 2023

Choose a reason for hiding this comment

reinrich Feb 15, 2023

Choose a reason for hiding this comment

openjdk bot commented Feb 15, 2023 • edited Loading

TheRealMDoerr left a comment

Choose a reason for hiding this comment

TheRealMDoerr Feb 15, 2023

Choose a reason for hiding this comment

TheRealMDoerr left a comment

Choose a reason for hiding this comment

reinrich commented Feb 17, 2023

openjdk bot commented Feb 17, 2023

openjdk bot commented Feb 17, 2023

reinrich commented Feb 14, 2023 •

edited by openjdk bot

Loading

mlbridge bot commented Feb 14, 2023 •

edited

Loading

openjdk bot commented Feb 15, 2023 •

edited

Loading