iximeow / yaxpeax-x86

x86 decoders for the yaxpeax project
BSD Zero Clause License
132 stars 23 forks source link

add RegSpec::rbx() helper #6

Closed chc4 closed 3 years ago

chc4 commented 3 years ago

there's RegSpec::ebx() but not rbx :(

chc4 commented 3 years ago

@iximeow Ok so there's a lot of missing ones, actually.

are the ones I noticed

Most of these exist in the realmode file, but some of them are missing there too, and esi/edi have notes that they're intentionally not public until 1.1. The constructors means you technically don't need them all since you can test equality, but RegSpec::b() etc. aren't const functions, so you can't match over them irrefutably (and RegisterBank isn't public so you can't construct a RegSpec constantly without them).

Do you want all of these constructors? Are they supposed to be "feature gated" behind 1.1 also? Is there a specific reason there isn't a macro like define_regs!(ax => W, 0; eax => D, 0; ) for them, to limit the copy-paste?

iximeow commented 3 years ago

Most of these exist in the real_mode file,

this is probably because protected_mode and real_mode were made by taking long_mode and making 64-bit parts into 32-bit or 16-bit as appropriate. so the omissions aren't "intentional", just that these accessors were for the most part written as i'd needed them for other parts of the decoder. i haven't ended up using them outside the crate which is why they're in a more rough spot.

but RegSpec::b(_) etc. aren't const functions

maybe it's better for them to be Option and return None in the out of bounds register number case, so they could be const... oh that's a major semver bump too. sigh. those need documentation they panic on invalid register numbers at the very least.

Is there a specific reason there isn't a macro

i'm macro averse and forget they can be useful. this is a really good idea.

edit: oh and your other questions; if you're offering to write them, it would be great to have consistent constructors for all the general-purpose registers. i'm less sure about the simd ones, but if there's a use case for them i could be convinced. if you want to make them all pub that's fine, there are some other changes that will roll up into a 1.1 bump too. esi and edi are because they were added in service of a patch update involving cmps and other string instructions, doubt there will be more of those in 1.0.