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

JitArm64_RegCache: Interface cleanup #9396

Merged
merged 6 commits into from Jan 1, 2021
Merged

Conversation

lioncash
Copy link
Member

Tidies up the interface in various minor ways, such as:

  • Marking member funcs as const when they don't modify internal state
  • Converting enums to enum classes
  • In-class initializing member variables where applicable.

Converting the RegType enum over to an enum class in particular uncovered an odd quirk in two places. RegType enum members were being passed as the second argument to BindToRegister here and here which is very unusual, as the second member of BindToRegister is a bool, not a RegType, which seems pretty sketchy. I don't know if this is an actual bug rather than identifiers being abused however, so I'd appreciate input on that.

Prevents namespace pollution and makes the enum members strongly typed.
This also mirrors the x64 variant as well.
Same behavior, but in-place initializes all values and makes it visually
explicit at the declaration site.
Also provides operator!= for logical symmetry.

We can also take the arguments by value, as the arguments are trivially
copyable enum values which fit nicely into registers already.
Also moves comments to make for less wonky formatting.
Many of these don't modify member state, so they can be marked as const.
Avoids polluting the namespace and makes the members of the enumeration
strongly typed.
Copy link
Member

@BhaaLseN BhaaLseN left a comment

Choose a reason for hiding this comment

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

Enum classes feel a lot nicer for those. Untested.

Copy link
Member

@leoetlino leoetlino left a comment

Choose a reason for hiding this comment

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

The REG_REG thing seems to be an oversight... It was added as part of bcde1aa#diff-bc54c6351f0ce20b932a4aa98992e19a69cdc5c55d5be79f839b25c8bf4b6d7cL132 which changed a lot of fpr.R(..., false); to fpr.R(..., REG_REG);. I bet it's a find-and-replace error :P

@leoetlino leoetlino merged commit 344a74a into dolphin-emu:master Jan 1, 2021
10 checks passed
@lioncash lioncash deleted the arm branch January 1, 2021 04:21
Gamer64ytb added a commit to Gamer64ytb/Dolphin-MMJR that referenced this pull request Feb 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants