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
8274730: AArch64: AES/GCM acceleration is broken by the fix for JDK-8273297 #5819
Conversation
|
@theRealAph The following label will be automatically applied to this pull request:
When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command. |
Webrevs
|
@theRealAph This change now passes all automated pre-integration checks. After integration, the commit message for the final commit will be:
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 27 new commits pushed to the
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.
|
Looks good with just 2 small comments.
I will test changes and approve it after that.
subkeyHtbl_48_entries_start | ||
= array_element_address(subkeyHtbl_48_entries, intcon(0), T_LONG); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please, don't split such line.
@@ -6794,14 +6794,20 @@ bool LibraryCallKit::inline_galoisCounterMode_AESCrypt() { | |||
ciKlass* klass = ciTypeArrayKlass::make(T_LONG); | |||
Node* klass_node = makecon(TypeKlassPtr::make(klass)); | |||
|
|||
// htbl entries is set to 96 only fox x86-64 | |||
// Does this target support this intrinsic? | |||
if (Matcher::htbl_entries == -1) return false; | |||
|
|||
// new array to hold 48 computed htbl entries |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move this comment under (Matcher::htbl_entries != 0)
check.
/integrate |
Going to push as commit c74726d.
Your commit was automatically rebased without conflicts. |
@theRealAph Pushed as commit c74726d. |
The recent AES/GCM acceleration on AArch64 was broken by https://bugs.openjdk.java.net/browse/JDK-8273297 . This was entirely expected, and I approved the patch, but now we must make AArch64 acceleration work again.
The only significant change from the point of view of this patch is that one argument was added to the call to the intrinsic, and that argument caused another argument to spill onto the stack.
Progress
Issue
Reviewers
Reviewing
Using
git
Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk pull/5819/head:pull/5819
$ git checkout pull/5819
Update a local copy of the PR:
$ git checkout pull/5819
$ git pull https://git.openjdk.java.net/jdk pull/5819/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 5819
View PR using the GUI difftool:
$ git pr show -t 5819
Using diff file
Download this PR as a diff file:
https://git.openjdk.java.net/jdk/pull/5819.diff