Implement basic inline asm support #72
Conversation
I'm loosely working on getting the asm working (something doesn't work as expected) and covering it with tests ATM, so go ahead. May I ask how you debug the backend? I'd like to be able to take a look at the intermediate representation gccjit builds, any way to dump it? Doesn't look like |
|
To dump the intermediate representations, there's a bunch of environment variables you can set as you can see here. |
|
You can try 2 things:
|
We don't support it yet at all
Turns out that `+` readwrite output registers cannot be tied with input variables.
|
I apologize for my silent disappearance, had to take an emergency trip to Siberia (poor comms), forgot to warn you in the chaos. I'm sorry. I may be forced to take a few more in the next a few days, but I'll try to warn you so you won't have to wait for me. Now to the matter at hand.
You did add it! I just missed it trying to search for "dump" in The function in question is Now that I've found the error, I'm trying to test if the code actually works, adding a snippet to The next thing I'm going to work on is working build and test system, that's settled. Bottom line: I think we'll have to support the |
|
No worries. I still haven't had the time to implement register variables anyway. Yes, this is fine for The tests in the For this error |
I swear I had them working at some point, but I forgot what exactly I did :( For now, I'll add a new "example".
I'll look into it, but I'd rather leave them out of this PR. It's sizeable enough |
|
Oh, actually the tests indeed run. (They didn't run in my custom branch on which I was working locally.) I added them in the CI in #73 . Oh, I thought you had the global init error because of your changes. So, it's caused by something else? |
I'm not really sure. I'm 99% sure that no changes to I'll try to checkout to master and see if |
|
HA!
And I do have |
|
Seems like How do you get this error? I'd need more details to help you. |
|
Also, it seems there are 3 new failing UI tests in the CI for this branch. There are 104 failure in this PR while yours have 107. |
# your latest commit before the last PR
# everything below is done at this commit
git checkout 6f50986667debbfc67776304a8ee23fe0158613f
# my libgcc path
echo -n /home/user/dev/gccjit/build/gcc > gcc_path
./clean_all.sh
# taken from CI
./prepare_build.sh && ./build.sh && cargo test
That was my guess too, but here's the error. My gcc_path is Running Makes sense. Now let's add Here we go again. At least now I know it wasn't my change that broke it. Ugh. I'll figure it out tomorrow. |
Not at all. I'll see tomorrow. |
|
Yeah, you need to set |
|
I added support for register variables in this commit. (here for the rust binding) |
|
I've figured why I couldn't run the tests and I added support for explicit registers. I'm on mobile now, will report in an hour. |
|
(Got distracted again, sorry). OK, now I can finally confirm that In order to run the tests you need to build the
Moderate issues:
Minor issues:
Unresolved questions:
Resolved questions:
|
|
I indeed did not implement the dump yet and those tests are indeed expected to be compiled using LLVM (sorry if it wasn't clear). For the stack alignment, as far as I know, it is for SSE code to work. The LLVM doc mentions "The compiler should make conservative assumptions about what the asm might contain and should generate its usual stack alignment code in the prologue if the ‘alignstack’ keyword is present", but it's unclear to me how to do that in GCC. I'll look at the rest later. Thanks for your work! |
Those are The first two use The last one uses |
There was a problem hiding this comment.
Nice work!
I'm half-way through reviewing this file, but I'm sending you those comments now.
By the way, the tests directory was only meant as a way to start testing when I couldn't run Rust tests suite. It's okay that you used it in this PR, but I don't think we should make it test the whole language. If anything is missing in the Rust tests suite, I believe it would be better to add it there.
Nothing immediately addressable:
I think we should merge this in as is - it's in pretty good shape IMHO. |
|
I've now excluded the |
|
For your information: https://gcc.gnu.org/pipermail/gcc-help/2021-September/140679.html I guess we need to bring it up with rustc devs and see what we can do. I don't think something like this would be a good solution: push rsp
and rsp, 0xffffffffffffff00
/* Code */
pop rspBecause this push would be useless in 99% cases. |
|
Yeah, I looked at the function prelude code generation in gcc, and while I couldn't find the one for x86, it looks like the stack alignment code is not separated from it, so it would be impossible to only call it. |
|
Thanks a lot for your amazing work! |
I decided to explicitly separate the declaration of out variables from the asm block generation, so now we first generate the output variables, then register variables (as soon as
gccjitadds support for them), then the asm block, then output operands, input operands, and clobbers. This system is simpler and less error prone, and also it should be easier to maintain and extend.I did my best to document the system thoroughly.
I've concentrated on X86 for now, barring SSE, AVX and so on (the rest of the backend doesn't support yet them anyway). Adding support for other architectures can ideally be done simply by specifying right constraints in the match branches.
I resolved most of the TODOs in the code, and added a few of my own.
Important questions we still need to resolve:
.intel_syntax noprefixhack might breakmmemory constraints. I believe the only realistic way forward is to make gccjit aware of the att/intel dialect difference. I know that you can set the dialect globally (per file) but I'm not really sure if dialect can be set per block. Does gcc even support it? Worst case scenario: we might need to forbid different dialects in one project.nostackis not specified?And of course I'm waiting for gccjit to add register variables. Meanwhile, I'll start testing if the system is working correctly.