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

Use fm's new matching features for x86 register matching. #1311

Merged
merged 1 commit into from
Jul 24, 2024

Conversation

ltratt
Copy link
Contributor

@ltratt ltratt commented Jul 23, 2024

Instead of using concrete register names, one can refer to a class of registers e.g. r.8 is all 8-bit registers. To match a register name but ignore its value uses r.8._: to match a register name use r.8.x. Suffixes: must be unique (i.e. r.8.x and r.8.y must refer to different registers); and they refer to the "underlying" register (e.g. r.8.x and r.64.x might match al and RAX which is considered the same register, but will fail to match against al and RBX).

To make this concrete these asserts all succeed:

let fmm = fmatcher("r.8.x r.8.y r.64.x");
assert!(fmm.matches("al bl rax").is_ok());
assert!(fmm.matches("al al rax").is_err());
assert!(fmm.matches("al bl rbx").is_err());
assert!(fmm.matches("al bl eax").is_err());

This commit is unfinished in the sense that not all the tests are ported: this is an RFC in its current state. Needs softdevteam/fm#50.

@vext01
Copy link
Contributor

vext01 commented Jul 23, 2024

The user-observable functionality looks great, but I don't feel like I've understood name_matching_processor() sufficiently to do more than a superficial review (I've often struggled with understanding name matchers in general, but I usually manage to muddle through).

If you are OK with that, then we can proceed.

@ptersilie
Copy link
Contributor

e.g. r.8.x and r.64.x might match al and RAX which is considered the same register, but will fail to match against al and RBX

Does this mean that r.8.x and r.64.x will only match if the register name is the same, even if the sizes are different. So the x part will bind to the A part of AL, RAX, etc?

@vext01
Copy link
Contributor

vext01 commented Jul 23, 2024

My understanding is that r.8.x would never match a r.64.x. So al and rax are treated as distinct. I think that's the way we'd want it anyway.

@ptersilie
Copy link
Contributor

ptersilie commented Jul 23, 2024

My understanding is that r.8.x would never match a r.64.x. So al and rax are treated as distinct. I think that's the way we'd want it anyway.

Yes, they are distinct, but the x binds all other sizes to the RAX register. So r.16.x needs to be ax, and r.32.x needs to be eax. They can't suddenly be ebx.

@ltratt
Copy link
Contributor Author

ltratt commented Jul 23, 2024

Does this mean that r.8.x and r.64.x will only match if the register name is the same, even if the sizes are different. So the x part will bind to the A part of AL, RAX, etc?

Yes, though it's not done by text but by the underlying register: so if r.8.x is al then r.64.x can only match rax (and not rbx etc).

@ltratt
Copy link
Contributor Author

ltratt commented Jul 23, 2024

Ported all remaining tests (including floating point register support) in 8d4a343. Hopefully ready for squashing!

@vext01
Copy link
Contributor

vext01 commented Jul 24, 2024

If @ptersilie is OK with this, I think I am.

@ltratt
Copy link
Contributor Author

ltratt commented Jul 24, 2024

@ptersilie OK to squash?

@ptersilie
Copy link
Contributor

Please squash.

Instead of using concrete register names, one can refer to a class of
registers e.g. `r.8` is all 8-bit registers. To match a register name
but ignore its value uses `r.8._`: to match a register name use `r.8.x`.
Suffixes: must be unique (i.e. `r.8.x` and `r.8.y` must refer to
different registers); and they refer to the "underlying" register (e.g.
`r.8.x` and `r.64.x` might match `al` and `RAX` which is considered the
same register, but will fail to match against `al` and `RBX`).

To make this concrete these `assert`s all succeed:

```rust
let fmm = fmatcher("r.8.x r.8.y r.64.x");
assert!(fmm.matches("al bl rax").is_ok());
assert!(fmm.matches("al al rax").is_err());
assert!(fmm.matches("al bl rbx").is_err());
assert!(fmm.matches("al bl eax").is_err());
```
@ltratt
Copy link
Contributor Author

ltratt commented Jul 24, 2024

Squashed.

@ptersilie ptersilie added this pull request to the merge queue Jul 24, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jul 24, 2024
@ptersilie ptersilie added this pull request to the merge queue Jul 24, 2024
Merged via the queue into ykjit:master with commit 663896e Jul 24, 2024
2 checks passed
@ltratt ltratt deleted the register_name_matching branch July 24, 2024 12:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants