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

8255437: [vector] jdk/incubator/vector/SelectFromByteMaxVectorTests fails with Arm SVE 2048-bits #19

Closed

Conversation

@XiaohongGong
Copy link
Collaborator

@XiaohongGong XiaohongGong commented Oct 29, 2020

The tests are used to test the Vector API "selectFrom". It uses index values stored in the lanes of this vector which is a byte vector for byte type. So its valid lane elements should be 0 ~ 127 for byte.

However, for SVE 2048-bits, the max vector length for byte is 256, which exceeds the byte boundary. Indexes greater than 127 are converted to the negative values which are invalid for the API.

To avoid this, it's better to guarantee the elements in the byte vector to the valid values if they are prepared to be used as vector indexes.

See more details from:
https://mail.openjdk.java.net/pipermail/panama-dev/2020-October/011216.html


Progress

  • Change must not contain extraneous whitespace
  • Change must be properly reviewed

Testing

Linux x32 Linux x64 Windows x64 macOS x64
Build ✔️ (1/1 passed) ✔️ (5/5 passed) (2/2 failed) ✔️ (2/2 passed)
Test (tier1) ✔️ (9/9 passed) ✔️ (9/9 passed)

Failed test tasks

Issue

  • JDK-8255437: [vector] jdk/incubator/vector/SelectFromByteMaxVectorTests fails with Arm SVE 2048-bits

Reviewers

Download

$ git fetch https://git.openjdk.java.net/panama-vector pull/19/head:pull/19
$ git checkout pull/19

@bridgekeeper
Copy link

@bridgekeeper bridgekeeper bot commented Oct 29, 2020

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

@openjdk openjdk bot added the rfr label Oct 29, 2020
@mlbridge
Copy link

@mlbridge mlbridge bot commented Oct 29, 2020

Webrevs

@XiaohongGong XiaohongGong changed the title 8255437: [vector] jdk/incubator/vector/SelectFromByteMaxVectorTests f… 8255437: [vector] jdk/incubator/vector/SelectFromByteMaxVectorTests fails with Arm SVE 2048-bits Oct 29, 2020
@openjdk
Copy link

@openjdk openjdk bot commented Oct 29, 2020

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

8255437: [vector] jdk/incubator/vector/SelectFromByteMaxVectorTests fails with Arm SVE 2048-bits

Reviewed-by: psandoz

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 vectorIntrinsics 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. Possible candidates are the reviewers of this PR (@PaulSandoz) but any other Committer may sponsor as well.

➡️ 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 the ready label Oct 29, 2020
@PaulSandoz
Copy link
Member

@PaulSandoz PaulSandoz commented Oct 30, 2020

Latest changes are fine, but you can just rename the m argument of the lambda expression to upper and make it even simpler :-) up to you, no need for another round of review.

@XiaohongGong
Copy link
Collaborator Author

@XiaohongGong XiaohongGong commented Nov 2, 2020

@PaulSandoz Thanks for looking at it again and thanks for your advice! I'd like to keep it as it is. Thanks again!

@XiaohongGong
Copy link
Collaborator Author

@XiaohongGong XiaohongGong commented Nov 2, 2020

/integrate

@openjdk openjdk bot added the sponsor label Nov 2, 2020
@openjdk
Copy link

@openjdk openjdk bot commented Nov 2, 2020

@XiaohongGong
Your change (at version d6811a4) is now ready to be sponsored by a Committer.

@nsjian
Copy link
Collaborator

@nsjian nsjian commented Nov 2, 2020

/sponsor

Note: a force-push will break review history.

@openjdk openjdk bot closed this Nov 2, 2020
@openjdk
Copy link

@openjdk openjdk bot commented Nov 2, 2020

@nsjian @XiaohongGong Pushed as commit bccabae.

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

@XiaohongGong XiaohongGong deleted the JDK-8255437 branch Nov 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
3 participants