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

Basic Inline Assembly #149

Merged
merged 3 commits into from
Dec 4, 2020
Merged

Basic Inline Assembly #149

merged 3 commits into from
Dec 4, 2020

Conversation

water111
Copy link
Collaborator

@water111 water111 commented Dec 4, 2020

This still needs some tests and some work around register allocation.

To get a "register", you can define it inside rlet, which is like a normal let but:

  • The initial value is optional. If none is provided, you get whatever happens to be in the register.
  • You can specify the type (optional). By default it's the type of the initial value, or object if there's no initial value.
  • You can specify the register (optional).
  • You can specify the register "class" (gpr/xmm), (optional)

It will behave like a normal variable and use normal GOAL coloring tricks, but can only live in that given register and can't be spilled to the stack ever. If it goes dead inside the rlet, the GOAL coloring system can use it as a temporary in any high level GOAL code. If you don't want this to happen, don't write higher level GOAL code in your rlet. This is usually not an issue because asm ops should interact correctly with the coloring system by default.

For even more control, use an asm-func a GOAL function where you have to do everything yourself. There's no prologue or epilogue. You can still write GOAL code, but the compiler shouldn't use the stack or any callee saved registers, and as a result some stuff can fail if you run out of temporary registers.

We should consider a "nuclear option" to completely bypass the coloring system and access registers. In either case, this is a dangerous thing to do and can mess with any higher level GOAL expression. Currently this is by setting :color #f inside of the asm operation, but it isn't clear that it works in a useful way yet.

It's not clear what should happen if you manually use a callee-saved register. I think it should by default work with the coloring system. It's too annoying to have a special mode for these where they can be used, but not default allocated, and not backed up only in asm-funcs. So it'll get backed up in a normal function, and error in an asm-function (unless you use the :color #f option).

Test ideas:

  • Support accessing registers that are "forbidden" in coloring (rsp, r15/offset)
  • Allow constraining a register and not using it (it's annoying when debugging if this errors)

@coveralls
Copy link

coveralls commented Dec 4, 2020

Pull Request Test Coverage Report for Build 401224861

  • 246 of 307 (80.13%) changed or added relevant lines in 17 files are covered.
  • 2 unchanged lines in 1 file lost coverage.
  • Overall coverage increased (+0.006%) to 81.008%

Changes Missing Coverage Covered Lines Changed/Added Lines %
goalc/emitter/Register.h 1 2 50.0%
goalc/compiler/Util.cpp 5 8 62.5%
goalc/regalloc/IRegister.cpp 0 3 0.0%
goalc/compiler/compilation/Function.cpp 14 18 77.78%
goalc/compiler/Compiler.cpp 10 15 66.67%
goalc/compiler/IR.cpp 71 76 93.42%
goalc/compiler/Compiler.h 0 7 0.0%
goalc/regalloc/Allocator.cpp 18 27 66.67%
goalc/compiler/CodeGenerator.cpp 18 28 64.29%
goalc/compiler/compilation/Asm.cpp 79 93 84.95%
Files with Coverage Reduction New Missed Lines %
goalc/emitter/Register.h 2 85.71%
Totals Coverage Status
Change from base Build 397318107: 0.006%
Covered Lines: 17437
Relevant Lines: 21525

💛 - Coveralls

@water111
Copy link
Collaborator Author

water111 commented Dec 4, 2020

I think I finally found a solution I like. It's heavily biased toward places where we'll need to do MIPS -> x86 assembly translation, but still allows us to very easily access non-standard registers or do things manually/without the stack for a few kernel functions. The goal doc has been updated to describe this behavior in more detail.

@water111 water111 marked this pull request as ready for review December 4, 2020 17:56
@water111 water111 merged commit 90e5c02 into master Dec 4, 2020
@water111 water111 deleted the w/inline-asm branch December 4, 2020 17:57
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.

2 participants