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

Add zNext support #7525

Merged
merged 8 commits into from
Jan 30, 2025
Merged

Add zNext support #7525

merged 8 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 and its
exploitation.

  1. Add processor recognition and facility checks for zNext
  2. Add zNext instructions
  3. Supporting changes for emulating zNext instructions on older IBM Z hardware.
  4. Use load indexed address instructions on zNext for aladd and aiadd
    -Adds support for generating load indexed address instructions for the
    add tree in axaddEvaluator when the tree has the correct shape.
  5. Add use of Count leading and trailing zeros instruction on zNext
    • For inotz/lnotz/inolz/lnolz nodes, use these new zNext instruction.
  6. Supporting changes to accelerate ExternalDecimal.checkExternalDecimal in
    OpenJ9.

@r30shah
Copy link
Contributor Author

r30shah commented Nov 5, 2024

FYI @dchopra001 @VermaSh @Spencer-Comin

@r30shah
Copy link
Contributor Author

r30shah commented Nov 8, 2024

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

This PR should be merge with OpenJ9 PR : eclipse-openj9/openj9#20507

@0xdaryl
Copy link
Contributor

0xdaryl commented Nov 11, 2024

I can review for general structural correctness, but for the actual architectural changes I will defer to Joran's expertise.

*/
bool yankIndexScalingOp()
{
return false;
Copy link
Contributor

Choose a reason for hiding this comment

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

This should probably return arch < zNext or check for LXA support instead of always returning false

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will update this.

Copy link
Contributor

Choose a reason for hiding this comment

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

@r30shah : Does @Spencer-Comin's comment still need to be added w/ the zNext check?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@joransiu @Spencer-Comin I added a codegen flag in 0b0086b to be set if the LXA instruction is available (zNext or can emulate those) to be used throughout.

@r30shah
Copy link
Contributor Author

r30shah commented Nov 15, 2024

Hi @joransiu / @0xdaryl ping to get reviews on this PR.

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

@joransiu
Copy link
Contributor

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

Sorry for the delay @r30shah. I'm looking at it now.

Copy link
Contributor

@joransiu joransiu left a comment

Choose a reason for hiding this comment

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

Changes LGTM besides the outstanding comment from Spencer.

*/
bool yankIndexScalingOp()
{
return false;
Copy link
Contributor

Choose a reason for hiding this comment

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

@r30shah : Does @Spencer-Comin's comment still need to be added w/ the zNext check?

Copy link
Contributor

@joransiu joransiu left a comment

Choose a reason for hiding this comment

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

LGTM

@0xdaryl
Copy link
Contributor

0xdaryl commented Jan 25, 2025

Please fix the merge conflicts.

Spencer-Comin and others added 7 commits January 27, 2025 09:40
Changes in this commit adds support for zNext instructions as well as
adds processor recognition and facility checks.

Signed-off-by: Spencer Comin <[email protected]>
Emulating the instructions when support is not availble on hardware
allows testing of exploitation that uses zNext instructions on older
IBM Z hardware. Changes in this commits adds necessary changes for
emulating zNext instructions.

Signed-off-by: Spencer Comin <[email protected]>
Adds support for generating load indexed address instructions for the
add tree in axaddEvaluator when the tree has the correct shape.

Signed-off-by: Spencer Comin <[email protected]>
Changes in this commit adds exploitation of Count leading and trailing
zeros on zNext for opcodes i/lnolz and i/lnotz respectively.
Apart from using the zNext instructions for these opcodes, this commit
also adds supporting changes to enable zNext emulation for these zNext
instructions.

Signed-off-by: Dhruv Chopra <[email protected]>
Accelerate Integer.expand, Integer.compress, Long.expand, and
Long.compress using the BDEPG and BEXTG instructions in zNext.

Signed-off-by: Spencer Comin <[email protected]>
Moving canEmulate enables us to check if emulation is enabled and if the
instruction op code supports emulation through a single call.

Signed-off-by: Shubham Verma <[email protected]>
Accelerate DAA ExternalDecimal.checkExternalDecimal api for verifying
Zoned/External decimals using zNext instruction, Vector Test Zoned. This
commit also includes changes required for generating Vector Test Zoned
instruction.

Signed-off-by: Shubham Verma <[email protected]>
Load indexed address register instructions are used to generate address to
access element in the array. This commit adds a code-gen query for using LXA
instruction to used at various places while doing pattern matching.

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

r30shah commented Jan 27, 2025

Looking into the changes - I see that in PR - #7603 DAA related changes were moved to OpenJ9 which was causing the merge conflict. I have resolved this from this PR and looking to port that to OpenJ9.

@0xdaryl
Copy link
Contributor

0xdaryl commented Jan 29, 2025

Jenkins build all

@0xdaryl 0xdaryl merged commit 0d22cef into eclipse-omr:master Jan 30, 2025
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants