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.

Sign up for GitHub

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

Jump to bottom

Add zNext support #20507

Merged
merged 6 commits into from
Jan 30, 2025
Merged

Add zNext support #20507

merged 6 commits into from
Jan 30, 2025

Conversation

r30shah
Copy link
Contributor

@r30shah r30shah commented Nov 5, 2024

Changes in this PR adds following supporting changes for zNext processor exploitation.

  1. Add zNext facility checks.
  2. Add infrastructure for emulating zNext instruction on older hardware.
  3. Adds emulation for following zNext instructions
    • Load Logical Indexed Address
    • Load Indexed Address
    • Bit deposit and Bit Extract (BDEPG/BEXTG)
    • CLZG/CTZG
  4. Accelerate Integer and Long expand and compress methods
    • Accelerate Integer.expand, Integer.compress, Long.expand, and
      Long.compress. These methods map directly to the BDEPG and BEXTG
      instructions in zNext.
  5. Accelerate ExternalDecimal.checkExternalDecimal
    • Accelerate DAA API for checkExternalDecimal using VTZ instruction.

@r30shah
Copy link
Contributor Author

r30shah commented Nov 5, 2024

FYI @dchopra001 @VermaSh @Spencer-Comin

@r30shah r30shah marked this pull request as ready for review November 6, 2024 20:35
@r30shah r30shah requested a review from dsouzai as a code owner November 6, 2024 20:35
@r30shah
Copy link
Contributor Author

r30shah commented Nov 8, 2024

@joransiu @0xdaryl Can we please get review on these changes ?

IBM SDK8 Build with these changes : /build_info.php?build_id=81048

Semeru JDK11/JDK21 build : view/OpenJ9 - Personal/job/Pipeline-Build-Test-Personal/24972/ - In progress.

@r30shah r30shah mentioned this pull request Nov 8, 2024
Merged
joransiu
joransiu requested changes Nov 13, 2024
View reviewed changes
@r30shah r30shah force-pushed the zNextSupportPR branch 3 times, most recently from 01b9c78 to 93008c2 Compare November 14, 2024 19:06
@r30shah
Copy link
Contributor Author

r30shah commented Nov 15, 2024

@joransiu last set of recommendations were addressed. Can I request for another review?

@r30shah
Copy link
Contributor Author

r30shah commented Jan 10, 2025

@joransiu @0xdaryl Can I request a round of review on these changes ? I have rebased the branch with to latest master.

0xdaryl
0xdaryl approved these changes Jan 21, 2025
View reviewed changes
Copy link
Contributor

@0xdaryl 0xdaryl left a comment

Choose a reason for hiding this comment

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

I was largely looking for structural issues. I'll defer to Joran's review for the zMetis details.

@0xdaryl
Copy link
Contributor

0xdaryl commented Jan 21, 2025

Does this PR have any dependencies on eclipse-omr/omr#7525, either breaking or otherwise?

@r30shah
Copy link
Contributor Author

r30shah commented Jan 21, 2025

Does this PR have any dependencies on eclipse-omr/omr#7525, either breaking or otherwise?

Yes, there is a dependency and we need to get this and OMR PR merged together (Given the area of the code these two PR touches).

@0xdaryl 0xdaryl self-assigned this Jan 21, 2025
@0xdaryl
Copy link
Contributor

0xdaryl commented Jan 24, 2025

Jenkins test sanity.functional,sanity.jdk zlinux,zos jdk21 depends eclipse-omr/omr#7525

@0xdaryl
Copy link
Contributor

0xdaryl commented Jan 25, 2025

Jenkins test sanity.functional,sanity.openjdk zlinux,zos jdk21 depends eclipse-omr/omr#7525

@0xdaryl
Copy link
Contributor

0xdaryl commented Jan 25, 2025

Jenkins test sanity.functional,sanity.openjdk zlinux jdk21 depends eclipse-omr/omr#7525

@r30shah
Copy link
Contributor Author

r30shah commented Jan 27, 2025

Given the merge-conflict in OMR PR, I have made one more change as part of DAA exploitation work to port the required changes from OMR to OpenJ9 - As it fits with DAA acceleration work, I have squashed the change with that commit.

@VermaSh Can I request you to take a look at the change https://github.com/eclipse-openj9/openj9/compare/1a81ab2940ed8f422baab14c560c82f8e7c71f5d..810296f5bd5eae02d937c8e82cd1b119edf9f5b4 to ensure those are the only changes needed.

Take a look at the comment eclipse-omr/omr#7525 (comment)

@VermaSh
Copy link
Contributor

VermaSh commented Jan 27, 2025

@r30shah That seems to have all needed changes.

@r30shah
Copy link
Contributor Author

r30shah commented Jan 27, 2025

@r30shah That seems to have all needed changes.

Thanks @VermaSh

@0xdaryl apologies for missing out the merge-conflict (Both OMR and OpenJ9 changes are rebased to latest upstream repo.

@0xdaryl
Copy link
Contributor

0xdaryl commented Jan 27, 2025

Jenkins test sanity.functional,sanity.openjdk zlinux jdk21 depends eclipse/omr@7525

@r30shah
Copy link
Contributor Author

r30shah commented Jan 27, 2025

Seems like build job failed https://openj9-jenkins.osuosl.org/job/Build_JDK21_s390x_linux_Personal/479/console 1 minute into starting without much helpful message for the reason. Launching another build.

@r30shah
Copy link
Contributor Author

r30shah commented Jan 27, 2025

Jenkins test sanity.functional,sanity.openjdk zlinux jdk21 depends eclipse-omr/omr#7525

@0xdaryl
Copy link
Contributor

0xdaryl commented Jan 28, 2025

Please see the build failure in the acceptance test build.

@r30shah r30shah force-pushed the zNextSupportPR branch 2 times, most recently from 275d2fe to add57df Compare January 28, 2025 11:37
@Spencer-Comin @r30shah
zNext facility checks
4314ea5 
Signed-off-by: Spencer Comin <[email protected]>
@Spencer-Comin @r30shah
Add emulation support for zNext instructions
17f5dda 
When running on non zNext hardware, emulating zNext instructions allows
testing the exploitation of zNext instructions. Following are the
summary of changes being made with this commit.

- Add infrastructure to emulate instructions
- Emulate following zNext instructions
  1. Load indexed address
  2. Bit deposit and extract
  3. Count leading and trailing zeros

Co-authored-by: Dhruv Chopra <[email protected]>
Co-authored-by: Shubham Verma <[email protected]>
Co-authored-by: Spencer Comin <[email protected]>

Signed-off-by: Spencer Comin <[email protected]>
@Spencer-Comin @r30shah
Accelerate Integer and Long expand and compress methods
aef9df0 
Accelerate Integer.expand, Integer.compress, Long.expand, and
Long.compress. These methods map directly to the BDEPG and BEXTG
instructions in zNext.

- Check if emulation for zNext instruction is enabled and if it is,
  enable the acceleration of compress and expand with emulation.

Signed-off-by: Spencer Comin <[email protected]>
@r30shah
Copy link
Contributor Author

r30shah commented Jan 28, 2025

Jenkins line endings check

VermaSh and others added 3 commits January 28, 2025 11:06
@VermaSh @r30shah
Accelerate DAA ExternalDecimal.checkExternalDecimal api
e3c72cf 
Accelerate DAA ExternalDecimal.checkExternalDecimal api for verifying
Zoned/External decimals using zNext instruction, Vector Test Zoned.
This change:
- Adds IL opCode for the ExternalDecimal.checkExternalDecimal
- Updates IL generator to recognize and inline
  ExternalDecimal.checkExternalDecimal
- Adds evaluator to accelerate the api call via Vector Test Zoned

Signed-off-by: Shubham Verma <[email protected]>
@r30shah
Address PR Comment: Add emulation support for zNext instructions
de59a99 
Signed-off-by: Rahil Shah <[email protected]>
@r30shah
Address PR Comment: Accelerate DAA ExternalDecimal.checkExternalDecim…
7366e65 
…al api

Signed-off-by: Rahil Shah <[email protected]>
@r30shah
Copy link
Contributor Author

r30shah commented Jan 28, 2025

Jenkins test sanity.functional,sanity.openjdk zlinux jdk21 depends eclipse-omr/omr#7525

@0xdaryl 0xdaryl merged commit c113b72 into eclipse-openj9:master Jan 30, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@VermaSh VermaSh VermaSh left review comments

@joransiu joransiu joransiu approved these changes

@0xdaryl 0xdaryl 0xdaryl approved these changes

@dsouzai dsouzai Awaiting requested review from dsouzai dsouzai is a code owner

Assignees

@0xdaryl 0xdaryl

Labels
comp:jit depends:omr:breaking
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@r30shah @0xdaryl @VermaSh @joransiu @Spencer-Comin