Skip to content

SME SVCR logic fix#339

Merged
FinnWilkinson merged 8 commits intodevfrom
sme-smstart-fix
Nov 2, 2023
Merged

SME SVCR logic fix#339
FinnWilkinson merged 8 commits intodevfrom
sme-smstart-fix

Conversation

@FinnWilkinson
Copy link
Copy Markdown
Contributor

This PR for the most part addresses issue #329 - correcting the SVCR and Streaming Mode logic introduced with SME. This is summaried as:

  • If a program is in Streaming Mode and calls SMSTART again, ZA and Vector registers are not zeroed out.
  • SVE Vector registers (and their aliases such as NEON and Scalar registers) are only zeroed out when SVCR.SM bit changes
  • SME's ZA register is only zeroed out when SVCR.ZA bit changes
  • SVCR can now be correctly updated using a regulat msr svcr, xt instruction in addition to smstart/smstop/msr svcr, #imm
  • More tests surrounding the SVCR logic have been added

Additional to this, logic for the NEON muti-struct store with post index instructions introduced in PR #335 has been corrected via a small change to the metadata to correctly identify if a #imm or xt is used as said post index.

NOTE: this branch is based on macos-sst-build-fix and thus contains the fixes present in PR #338. This PR will be rebased to dev once PR #338 has been merged and should be merged into dev after PR #338.

@FinnWilkinson FinnWilkinson added the bug Something isn't working label Oct 30, 2023
@FinnWilkinson FinnWilkinson self-assigned this Oct 30, 2023
@FinnWilkinson FinnWilkinson linked an issue Oct 30, 2023 that may be closed by this pull request
@JosephMoore25
Copy link
Copy Markdown
Contributor

Looks mostly good, but tiny code inconsistency.

You define "const Architecture& arch = instruction_.getArchitecture();" but then still use instruction_.getArchitecture() a couple lines later and then switch to arch a bit later. Probs best to just stick to one or the other for the sake of consistency.

Comment thread src/lib/arch/aarch64/ExceptionHandler.cc Outdated
ABenC377
ABenC377 previously approved these changes Oct 31, 2023
Copy link
Copy Markdown
Contributor

@ABenC377 ABenC377 left a comment

Choose a reason for hiding this comment

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

Can 'instruction_.getArchitecture()' in ExceptionHandler.cc line 661 be replaced with the 'arch' variable declared above it?

@FinnWilkinson
Copy link
Copy Markdown
Contributor Author

@JosephMoore25 @ABenC377 Good spot, will change this now

JosephMoore25
JosephMoore25 previously approved these changes Oct 31, 2023
Copy link
Copy Markdown
Contributor

@dANW34V3R dANW34V3R left a comment

Choose a reason for hiding this comment

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

One comment

Comment thread src/lib/arch/aarch64/Instruction.cc
@JosephMoore25 JosephMoore25 self-requested a review November 1, 2023 11:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Incorrect logic on multiple SMSTART calls

5 participants