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

8267368: Add masking support for reduction vector intrinsics #86

Closed

Conversation

@XiaohongGong
Copy link
Collaborator

@XiaohongGong XiaohongGong commented May 24, 2021

This patch adds the masking support for vector reduction operations, including the Vector API java implementation and hotspot intrinsics changes. It adds the mask information to the reduction intrinsic method. With it:

  • Both masked and non-masked reduction operations call the same hotspot intrinsics. The mask value and class are set to "null" for non-masked operations.
  • Reduction IR with mask as another input will be generated if the platform supports the predicate feature for the op. Otherwise, the vector blend pattern will be generated like before.

Note that this patch only contains the Vector API java implementation and the hotspot intrinsics changes. No compiler mid-end and backend implementations are included.


Progress

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

Issue

  • JDK-8267368: Add masking support for reduction vector intrinsics

Reviewers

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/panama-vector pull/86/head:pull/86
$ git checkout pull/86

Update a local copy of the PR:
$ git checkout pull/86
$ git pull https://git.openjdk.java.net/panama-vector pull/86/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 86

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.java.net/panama-vector/pull/86.diff

@bridgekeeper
Copy link

@bridgekeeper bridgekeeper bot commented May 24, 2021

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

@XiaohongGong
Copy link
Collaborator Author

@XiaohongGong XiaohongGong commented May 24, 2021

Hi @PaulSandoz @iwanowww , would you mind taking a look at the masking support part for the vector reduction operations? Thanks so much! Any feedback is welcome!

@openjdk openjdk bot added the rfr label May 24, 2021
@mlbridge
Copy link

@mlbridge mlbridge bot commented May 24, 2021

Webrevs

Copy link
Member

@PaulSandoz PaulSandoz left a comment

Looks good.

@openjdk
Copy link

@openjdk openjdk bot commented May 24, 2021

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

8267368: Add masking support for reduction vector intrinsics

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+mask 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.

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

@openjdk openjdk bot added the ready label May 24, 2021
@XiaohongGong
Copy link
Collaborator Author

@XiaohongGong XiaohongGong commented May 25, 2021

Looks good.

Thanks for your review @PaulSandoz !

@XiaohongGong
Copy link
Collaborator Author

@XiaohongGong XiaohongGong commented May 27, 2021

Hi @iwanowww, may I have your kindly feedback to the compiler side for this PR please? Thanks so much!

@XiaohongGong
Copy link
Collaborator Author

@XiaohongGong XiaohongGong commented Jun 9, 2021

Hi @iwanowww , any feedback to the compiler changes? Thanks so much!

@XiaohongGong
Copy link
Collaborator Author

@XiaohongGong XiaohongGong commented Jun 25, 2021

Hi @iwanowww , we'd like to integrate this PR first to promote the further masking feature development. Any comments and feedback are still welcome! And we can revisit them in future. Thanks so much!

Best Regards,
Xiaohong

@XiaohongGong
Copy link
Collaborator Author

@XiaohongGong XiaohongGong commented Jun 25, 2021

/integrate

@openjdk
Copy link

@openjdk openjdk bot commented Jun 25, 2021

Going to push as commit aff1e3d.

@openjdk openjdk bot closed this Jun 25, 2021
@openjdk openjdk bot added integrated and removed ready rfr labels Jun 25, 2021
@openjdk
Copy link

@openjdk openjdk bot commented Jun 25, 2021

@XiaohongGong Pushed as commit aff1e3d.

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

@XiaohongGong XiaohongGong deleted the JDK-8267368 branch Jun 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
2 participants