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

should work now (Nightly e4ca1662f 2021-05-22) #149

Merged
merged 1 commit into from
May 24, 2021
Merged

Conversation

Lokathor
Copy link
Member

@Lokathor Lokathor commented May 23, 2021

Closes #141

@Lokathor
Copy link
Member Author

@Lymia please double check that I adjusted the assembly correctly <3

@Lymia
Copy link
Contributor

Lymia commented May 24, 2021

Ah. The idea here is that the copy happens in one instruction either way, because (AFAIK) interrupts cannot happen in the middle of an instruction. Instead of adding pushes (which theoretically would allow an interrupt to happen between the push and the stmia), what you want to do instead is remove r9 from every case that has it, then just remove one case off the end, and make larger structs use the slow path instead.

@Lokathor
Copy link
Member Author

The new push/pop of r9 is just because we can't automatically trash r9 during our assembly block. LLVM itself might be using it and for reasons I don't fully follow inline-asm can't auto push/pop that register when necessary any more (zulip). The fact that we were allowed to use r9 before was a bug in inline-asm. Once they fixed that bug, we just have to save/restore the value of r9 ourselves.

However, my understanding is that even if we're saving and restoring that register manually instead of automatically, the fundamental logic of the near-atomic nature of a load-multi and then store-multi holds. An interrupt can't happen during a store-multi operation, so either it interrupts before the store-multi or after it, but the memory will be consistent either way.

@Lymia
Copy link
Contributor

Lymia commented May 24, 2021

Oh! Aha, I misunderstood what you did. You're saving and restoring r9 but leaving the actual store operation the same. Just saving it on the stack. I hadn't thought of doing that before, and it makes a great deal of sense. I like it. This should work perfectly.

@Lokathor
Copy link
Member Author

Also, why are we skipping over r6, r7, and r11 when picking registers for the load-multi/store-multi? Are they also special registers?

@Lymia
Copy link
Contributor

Lymia commented May 24, 2021

IIRC, I tried them and they gave me errors. I think so. For all I know, the PR has changed which registers are considered special, and some of them are available now.

@Lokathor
Copy link
Member Author

good enough! we can investigate that later.

@Lokathor Lokathor merged commit 7df5f47 into main May 24, 2021
@Lokathor Lokathor deleted the fix-the-asm-problems branch May 24, 2021 01:07
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.

Compilation fails with invalid register error
2 participants