Skip to content

Poly1305 AVX512 intrinsic for x86_64#1

Open
vpaprotsk wants to merge 30 commits intomasterfrom
avx512-poly
Open

Poly1305 AVX512 intrinsic for x86_64#1
vpaprotsk wants to merge 30 commits intomasterfrom
avx512-poly

Conversation

@vpaprotsk
Copy link
Copy Markdown
Owner

@vpaprotsk vpaprotsk commented Aug 31, 2022

No description provided.

vpaprotsk
vpaprotsk commented Sep 2, 2022
View reviewed changes
Comment thread src/java.base/share/classes/module-info.java Outdated
Comment thread src/java.base/share/classes/com/sun/crypto/provider/Poly1305.java Outdated
Comment thread src/java.base/share/classes/com/sun/crypto/provider/Poly1305.java Outdated
Comment thread src/java.base/share/classes/com/sun/crypto/provider/Poly1305.java Outdated
Comment thread src/java.base/share/classes/com/sun/crypto/provider/Poly1305.java Outdated
Comment thread src/hotspot/cpu/x86/assembler_x86.cpp Outdated
Comment thread src/hotspot/cpu/x86/assembler_x86.cpp Outdated
Comment thread src/hotspot/cpu/x86/assembler_x86.cpp Outdated
Copy link
Copy Markdown
Owner Author

@vpaprotsk vpaprotsk Sep 2, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check 32?

Comment thread src/hotspot/cpu/x86/assembler_x86.cpp Outdated
Copy link
Copy Markdown
Owner Author

@vpaprotsk vpaprotsk Sep 2, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FIXME x3

Comment thread src/hotspot/cpu/x86/assembler_x86.cpp Outdated
Copy link
Copy Markdown
Owner Author

@vpaprotsk vpaprotsk Sep 2, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

32 bit?

sviswa7
sviswa7 reviewed Sep 2, 2022
View reviewed changes
Copy link
Copy Markdown

@sviswa7 sviswa7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some initial comments

Comment thread src/hotspot/cpu/x86/assembler_x86.cpp Outdated
Comment thread src/hotspot/cpu/x86/assembler_x86.cpp Outdated
Copy link
Copy Markdown

@sviswa7 sviswa7 Sep 2, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/* no_mask_reg / true should be / no_mask_reg */ false

Comment thread src/hotspot/cpu/x86/assembler_x86.cpp Outdated
Comment thread src/hotspot/cpu/x86/assembler_x86.cpp Outdated
Comment thread src/hotspot/cpu/x86/assembler_x86.cpp Outdated
Comment on lines 5887 to 5891
Copy link
Copy Markdown

@sviswa7 sviswa7 Sep 2, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be under #ifdef _LP64, #endif.

Comment thread src/hotspot/cpu/x86/assembler_x86.cpp Outdated
Comment on lines 5902 to 5916
Copy link
Copy Markdown

@sviswa7 sviswa7 Sep 2, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be under #ifdef _LP64, #endif.

Comment thread src/hotspot/cpu/x86/assembler_x86.hpp Outdated
Copy link
Copy Markdown

@sviswa7 sviswa7 Sep 2, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be under #ifdef _LP64, #endif.

Comment thread src/hotspot/cpu/x86/assembler_x86.hpp Outdated
Copy link
Copy Markdown

@sviswa7 sviswa7 Sep 2, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be under #ifdef _LP64, #endif.

@vpaprotsk vpaprotsk changed the title poly clean Poly1305 AVX512 intrinsic for x86_64 Sep 2, 2022
ereshetova
ereshetova reviewed Sep 26, 2022
View reviewed changes
Comment thread src/java.base/share/classes/com/sun/crypto/provider/Poly1305.java Outdated
Copy link
Copy Markdown

@ereshetova ereshetova Sep 26, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this fixme note be removed or what exactly needs fixing here?

Copy link
Copy Markdown
Owner Author

@vpaprotsk vpaprotsk Sep 26, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I forgot to remove FIXME during cleanup. I believe I was leaving a note for myself to see if there were any off-by-one errors in the loop, but I checked all that with stepping with a debugger..

(Yes, it should be deleted. Well.. this whole block, from line 182)

@vpaprotsk vpaprotsk force-pushed the avx512-poly branch 3 times, most recently from 8abb6cc to d314b03 Compare October 4, 2022 17:14
@ascarpino
Copy link
Copy Markdown

ascarpino commented Oct 24, 2022

I will look at this

@vpaprotsk
Copy link
Copy Markdown
Owner Author

vpaprotsk commented Oct 24, 2022

I will look at this

@ascarpino I think you meant to comment on openjdk#10582

(I used this PR for some preliminary reviews and an internal security review..)

vpaprotsk pushed a commit that referenced this pull request Nov 13, 2024
8339794: Open source closed choice tests #1
5e5942a 
Reviewed-by: jdv, prr
vpaprotsk pushed a commit that referenced this pull request Nov 13, 2024
8340687: Open source closed frame tests #1
47fcf5a 
Reviewed-by: aivanov
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@ereshetova ereshetova ereshetova left review comments

@sviswa7 sviswa7 sviswa7 left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@vpaprotsk @ascarpino @ereshetova @sviswa7