Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8235753: [lworld] Handle deoptimization when buffering scalarized inline type args in C1 #6

Closed
wants to merge 1 commit into from

Conversation

@TobiHartmann
Copy link
Contributor

TobiHartmann commented Mar 27, 2020

When calling from C2 to C1 compiled code (i.e., no adapter in between) we need to buffer scalarized value type arguments in the C1 compiled entry point of the method (because C1 requires an oop). To do that, we might need to call into the runtime and that call might trigger deoptimization of the C1 compiled caller. That's a problem because the arguments are still scalarized and neither the deopt code nor the interpreter know how to handle that state. It's not a problem for all the other cases where buffering is necessary because it's either in the c2i adapter or C2 compiled code where we can handle this already.

After trying all my ideas listed in JBS, I've ended up with the following solution:
When deoptimization of a C1 frame that is currently in a runtime call to buffer_value_args_xxx is performed, don't patch the return address but only set the orig_pc in the frame (see changes to frame::deoptimize). This delays deoptimization until we enter the method body where a new runtime check added in LIRGenerator::do_Base will detect the pending deoptimization by checking the original_pc and jump to a deopt stub. This requires initializing the original_pc to zero at the beginning of the method entry (because it usually contains garbage).

The patch also fixes the following problems:

  • Stack banging code is accidentally skipped when entering the C1 method through a scalarized entry
  • A clinit_barrier is never needed if the method is non-static
  • Refactoring, conversion of some checks to asserts, better comments

Thanks,
Tobias


Progress

  • Change must not contain extraneous whitespace

Issue

  • JDK-8235753: [lworld] Handle deoptimization when buffering scalarized inline type args in C1

Download

$ git fetch https://git.openjdk.java.net/valhalla pull/6/head:pull/6
$ git checkout pull/6

…ine type args in C1
@bridgekeeper
Copy link

bridgekeeper bot commented Mar 27, 2020

👋 Welcome back thartmann! A progress list of the required criteria for merging this PR into lworld will be added to the body of your pull request.

@openjdk
Copy link

openjdk bot commented Mar 27, 2020

@TobiHartmann This change now passes all automated pre-integration checks, type /integrate in a new comment to proceed. After integration, the commit message will be:

8235753: [lworld] Handle deoptimization when buffering scalarized inline type args in C1
  • If you would like to add a summary, use the /summary command.
  • To credit additional contributors, use the /contributor command.
  • To add additional solved issues, use the /solves command.

Since the source branch of this PR was last updated there have been 4 commits pushed to the lworld branch. Since there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid automatic rebasing, please merge lworld into your branch, and then specify the current head hash when integrating, like this: /integrate 0595ad08114f86b1916a16351e57c9edc94e2945.

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

@openjdk openjdk bot added ready rfr labels Mar 27, 2020
@mlbridge
Copy link

mlbridge bot commented Mar 27, 2020

Webrevs

@mlbridge
Copy link

mlbridge bot commented Mar 31, 2020

Mailing list message from Roland Westrelin on valhalla-dev:

Webrev: https://webrevs.openjdk.java.net/valhalla/6/webrev.00

Ok.

Roland.

@TobiHartmann
Copy link
Contributor Author

TobiHartmann commented Mar 31, 2020

/integrate

@mlbridge
Copy link

mlbridge bot commented Mar 31, 2020

Mailing list message from Tobias Hartmann on valhalla-dev:

Thanks, Roland!

Best regards,
Tobias

On 31.03.20 15:32, Roland Westrelin wrote:

Webrev: https://webrevs.openjdk.java.net/valhalla/6/webrev.00

Ok.

Roland.

@openjdk openjdk bot closed this Mar 31, 2020
@openjdk openjdk bot added integrated and removed ready rfr labels Mar 31, 2020
@openjdk
Copy link

openjdk bot commented Mar 31, 2020

@TobiHartmann The following commits have been pushed to lworld since your change was applied:

  • ece2a2d: 8241918: [lworld] Build failures after JDK-8236522
  • 2a6ddbd: 8241910: [lworld] C2 crashes due to duplicate storestore barrier added by JDK-8236522
  • cab7a5b: 8236522: NonTearable marker interface for inline classes

Your commit was automatically rebased without conflicts.

Pushed as commit 0595ad0.

@mlbridge
Copy link

mlbridge bot commented Mar 31, 2020

Mailing list message from Tobias Hartmann on valhalla-dev:

Changeset: 0595ad0
Author: Tobias Hartmann <thartmann at openjdk.org>
Date: 2020-03-31 13:48:23 +0000
URL: https://git.openjdk.java.net/valhalla/commit/0595ad08

8235753: [lworld] Handle deoptimization when buffering scalarized inline type args in C1

! src/hotspot/cpu/x86/c1_LIRAssembler_x86.cpp
! src/hotspot/cpu/x86/c1_MacroAssembler_x86.cpp
! src/hotspot/cpu/x86/frame_x86.cpp
! src/hotspot/share/c1/c1_FrameMap.hpp
! src/hotspot/share/c1/c1_LIR.cpp
! src/hotspot/share/c1/c1_LIR.hpp
! src/hotspot/share/c1/c1_LIRAssembler.cpp
! src/hotspot/share/c1/c1_LIRAssembler.hpp
! src/hotspot/share/c1/c1_LIRGenerator.cpp
! src/hotspot/share/c1/c1_MacroAssembler.hpp
! src/hotspot/share/runtime/deoptimization.cpp
! src/hotspot/share/runtime/frame.cpp
! test/hotspot/jtreg/compiler/valhalla/valuetypes/TestDeoptimizationWhenBuffering.java

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

Successfully merging this pull request may close these issues.

None yet

1 participant
You can’t perform that action at this time.