-
Notifications
You must be signed in to change notification settings - Fork 4
[hec-assembler]: Fix suggested bank error #78
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
Conversation
Actively testing this for outliers currently |
Hi @joserochh , is this ready for revision? |
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.
LGTM
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.
Tested out and not seeing any regressions in functionality/performance beyond some execution time. LGTM
@joserochh I guess this is good to merge. It would be good to do so before Christopher rebases his branch, to avoid having to rebase this one again. |
Fixes suggested bank error Co-authored-by: Kylan Race <[email protected]> Co-authored-by: Flavio Bergamaschi <[email protected]>
Proposed changes
Fix suggested bank error by adding dependencies to eviction XStore instructions. This avoids instructions coming before a variable is allocated in correct bank.
Types of changes
What types of changes does your code introduce to the Encrypted Computing SDK project?
Put an
x
in the boxes that applyChecklist
Put an
x
in the boxes that apply. You can also fill these out after creatingthe PR. If you are unsure about any of them, do not hesitate to ask. We are
here to help! This is simply a reminder of what we are going to look for before
merging your code.