Skip to content

8309777: [lworld+vector] Fix re-materialize crash for vector objects with un-vectorized multi-fields during deoptimization #863

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

Closed
wants to merge 1 commit into from

Conversation

XiaohongGong
Copy link

@XiaohongGong XiaohongGong commented Jun 20, 2023

Some vector API jtreg tests crash when running on hardwares that do
not support the relative vector species.

Here is the log:

  A fatal error has been detected by the Java Runtime Environment:

  Internal Error (valhalla/src/hotspot/share/prims/vectorSupport.cpp:317), pid=2529357, tid=2529377
  assert(loc_type == Location::oop || loc_type == Location::narrowoop) failed: expected 'oop'(2) or 'narrowoop'(9) types location but got: 3

  JRE version: OpenJDK Runtime Environment (21.0) (fastdebug build 21-internal-git-62ba7fc27)
  Java VM: OpenJDK 64-Bit Server VM (fastdebug 21-internal-git-62ba7fc27, mixed mode, sharing, compressed oops, compressed class ptrs, g1 gc, linux-aarch64)
  Problematic frame:
  V [[libjvm.so](http://libjvm.so/)+0x1aed828] VectorSupport::allocate_vector_payload(InstanceKlass*, int, BasicType, frame*, RegisterMap*, ObjectValue*, JavaThread*)+0x41

The root cause is VectorSupport::allocate_vector_payload assumes the
ScopeValue which denotes the field value generated by C2 compiler
is a vector or an oop. But after the vector's payload is changed to be
the MultiField annotated field, the non-static fields of the vector
class is changed to be a series of fields with the same primitive type.
In C2 compiler, operations for such multi-fields can be vectorized only
if the running hardware supports the vector operations for the specified
vector size. For such cases, the field value generated by C2 is a vector.
Otherwise, the fields are the scalarized primitive type values. Hence
the re-materizalization for such vector objects should be handled just
like other normal objects.

To make things right when re-materializing vector objects whose fields
are not vectorized, two parts need to be modified:

  1. In c2 compiler, pass the un-vectorized multi-fields on safepoint when
    scalaring an InlineTypeNode. To try to vectorize the multi-fields in
    compiler, only the multi-field base is added to the klass's non-static
    field list. And the other multi-fields are saved in a secondary field list
    of their multi-field base. So if the multi-fields are not vectorized, all
    the field members should be found out and passed to the safepoint.

  2. During deoptimization, re-materialize the vector objects with different
    routines based on the ScopeValue types. That is, go to the VectorSupport
    routine to re-allocate the objects from a vector value, while go to the
    normal objects re-allocation path if the field value is not a vector.


Progress

  • Change must not contain extraneous whitespace

Issue

  • JDK-8309777: [lworld+vector] Fix re-materialize crash for vector objects with un-vectorized multi-fields during deoptimization (Enhancement - P4)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 863

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

Using diff file

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

Webrev

Link to Webrev Comment

…with un-vectorized multi-fields during deoptimization
@bridgekeeper
Copy link

bridgekeeper bot commented Jun 20, 2023

👋 Welcome back xgong! A progress list of the required criteria for merging this PR into lworld+vector 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 Jun 20, 2023

@XiaohongGong 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:

8309777: [lworld+vector] Fix re-materialize crash for vector objects with un-vectorized multi-fields during deoptimization

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 no new commits pushed to the lworld+vector branch. If another commit should be pushed before you perform the /integrate command, your PR will be automatically rebased. If you prefer to avoid any potential automatic rebasing, please check the documentation for the /integrate command for further details.

As you do not have Committer status in this project an existing Committer must agree to sponsor your change.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

@openjdk openjdk bot added ready Pull request is ready to be integrated rfr Pull request is ready for review labels Jun 20, 2023
@mlbridge
Copy link

mlbridge bot commented Jun 20, 2023

Webrevs

@XiaohongGong XiaohongGong mentioned this pull request Jun 25, 2023
1 task
@XiaohongGong
Copy link
Author

Since this issue is fixed by #866 which has been merged, close this PR now.

@XiaohongGong XiaohongGong deleted the deop-mf branch June 27, 2023 07:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready Pull request is ready to be integrated rfr Pull request is ready for review
Development

Successfully merging this pull request may close these issues.

1 participant