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

Add missing assembly instructions to the x86 backend #20050

Draft
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

Arnau478
Copy link
Contributor

@Arnau478 Arnau478 commented May 23, 2024

The x86 backend was missing some important instructions. They're not generated by the compiler, but having them ensures they can be used in inline assembly. Added tests for all of them. Control registers were also added. A list of added instructions:

  • rdmsr
  • wrmsr
  • sysret
  • cmc
  • clc
  • stc
  • cli
  • sti
  • cld
  • std
  • int1
  • int
  • iret
  • hlt

Fixes #19773

Arnau478 and others added 6 commits April 27, 2024 11:12
@andrewrk andrewrk requested a review from jacobly0 May 29, 2024 00:42
@sfiedler0
Copy link
Contributor

It would be a good idea to add the int instruction (which throws an interrupt and is really good for testing interrupt handling in the kernel).

@Arnau478
Copy link
Contributor Author

Arnau478 commented May 31, 2024

It would be a good idea to add the int instruction (which throws an interrupt and is really good for testing interrupt handling in the kernel).

That's true, int and iret were missing! Added them in b5d7804 and edited original comment

@Arnau478 Arnau478 changed the title Add missing assembly instruction to the x86 backend Add missing assembly instructions to the x86 backend May 31, 2024
@sfiedler0
Copy link
Contributor

Probably also some instructions related to inb and outb (and inw and so on)?
Port I/O is really important for kernels (UART logging, PIC communicating, PS/2 and whatever)

@Arnau478
Copy link
Contributor Author

Probably also some instructions related to inb and outb (and inw and so on)? Port I/O is really important for kernels (UART logging, PIC communicating, PS/2 and whatever)

The thing is, in/out instructions are kind of weird. Like rdmsr and wrmsr, they have implicit register operands. However, they are usually written with the operands being explicit (even though only al/ax/eax and dx are allowed, respectively). This wouldn't fit the current assembler implementation. Also, there are many ways to do it (size postfix, explicit register, or both), regarding language design. I think it should be made a separate issue and debated there. Either way, it requires a design decision that I hope someone with an important role in the organization can make.

Copy link
Member

@jacobly0 jacobly0 left a comment

Choose a reason for hiding this comment

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

Unfortunately, the CI does not run any tests in src, and the tests added in this PR do not pass as many of the added encodings are wrong. If you prefer, you can make these changes upstream first where the CI does run the tests. Also note that without Mir encodings most of these instructions will not be usable in inline asm, which should have been obvious if any amount of testing had been done before creating this PR. I'm afraid making these kind of changes requires a lot of attention to detail to get right.

@Arnau478
Copy link
Contributor Author

@jacobly0 I'm sorry, I missed the Mir. I will set the PR as a draft for now, in case someone wants to address the linked issue while I work on it. Thanks for mentioning the upstream.

@Arnau478 Arnau478 marked this pull request as draft June 16, 2024 18:14
@jacobly0
Copy link
Member

The thing is, in/out instructions are kind of weird. Like rdmsr and wrmsr, they have implicit register operands. However, they are usually written with the operands being explicit (even though only al/ax/eax and dx are allowed, respectively).

See stos/stosb etc for an example of how to implement this.

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.

Missing x86 mnemonics and registers (self-hosted backend encoder)
3 participants