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

Misc. x86 monenter/monexit cleanup and readability improvements #21101

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Misc. x86 monenter/monexit cleanup and readability improvements #21101

wants to merge 3 commits into from

Conversation

0xdaryl
Copy link
Contributor

@0xdaryl 0xdaryl commented Feb 11, 2025

  • Inline x86 monitor entry/exit evaluator improvements for readability
  • Remove experimental read only monitors on x86
  • Make assembly formatting consistent in X86LockReservation.nasm

@0xdaryl
Copy link
Contributor Author

0xdaryl commented Feb 11, 2025

Jenkins test sanity xlinux,win,osx jdk21

@0xdaryl
Copy link
Contributor Author

0xdaryl commented Feb 12, 2025

Jenkins test sanity xlinux,win,osx jdk21

@0xdaryl
Copy link
Contributor Author

0xdaryl commented Feb 13, 2025

Jenkins test sanity xlinux,win,osx jdk21

@0xdaryl
Copy link
Contributor Author

0xdaryl commented Mar 3, 2025

Jenkins test sanity xlinux,win,osx jdk21

@0xdaryl
Copy link
Contributor Author

0xdaryl commented Mar 5, 2025

Linux failure is #21182, except with JDK 21.

@0xdaryl
Copy link
Contributor Author

0xdaryl commented Mar 5, 2025

@BradleyWood : would you mind reviewing this please? There should be no functional or performance changes with this PR: only stylistic, readability, and deprecated code removal changes.

BradleyWood
BradleyWood reviewed Mar 5, 2025
View reviewed changes
Copy link
Member

@BradleyWood BradleyWood left a comment

Choose a reason for hiding this comment

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

Why did you leave these global declarations indented at 4 spaces?

https://github.com/0xdaryl/openj9/blob/5a4b81ab459df31253bba2b0c0f40aac3847fcd9/runtime/compiler/x/runtime/X86LockReservation.nasm#L23-L34

Everything else you moved to 6-spaces, is there a reason you decided to indent twice and not once? (where 1 tab = 3 spaces)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@BradleyWood BradleyWood BradleyWood left review comments

Assignees
No one assigned
Labels
comp:jit
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@0xdaryl @BradleyWood