Jump to conversation
Unresolved conversations (2)
@SchrodingerZhu SchrodingerZhu May 24, 2024
just curious, does `arm/aarch64` also support multi-versioned symbols?
libc/src/setjmp/arm/setjmp.cpp
nickdesaulniers smithp35
@frobtech frobtech May 23, 2024
I think the norm is to use `__asm__`. Consider using a raw string with natural newlines. The `!` is not really useful here, since `r0` is not used again. The syntax allows `{r4-r12, lr}` which IMHO is more readable. It's unusual to use the `.w` suffix rather than letting the assembler decide what encoding to use. I'd just use `ldm r0, {r4-r12, lr}` here.
Outdated
libc/src/setjmp/arm/longjmp.cpp
nickdesaulniers
Resolved conversations (9)
@statham-arm statham-arm Jun 3, 2024
I'm confident this is right, but I'm curious why you didn't do it the same way as in `longjmp`, by doing the original store in smaller blocks so that you don't clobber those registers in the first place. Set r1,r2,r3 to r8,r9,r10 and do a three-reg `stmia`; then set them to r11,sp,lr and do another one. One fewer instruction and less memory access. It's _more_ of a win here to do it like this than it was in `longjmp`, because you have one more low register free, so you can split the store into two 3-reg chunks instead of three 2-reg ones.
Outdated
libc/src/setjmp/arm/setjmp.cpp
statham-arm nickdesaulniers
@statham-arm statham-arm May 28, 2024
This approach will surely clobber the values that r4-r7 had on entry to `setjmp`? `setjmp` has to do two things with the values of callee-saved registers. It has to put them in the `jmp_buf` for `longjmp` to restore. But it _also_ has to preserve them itself, for when it returns directly to the caller.
Outdated
libc/src/setjmp/arm/setjmp.cpp
nickdesaulniers
@statham-arm statham-arm May 28, 2024
This use of r1 ought to be accessing the value passed in to `longjmp` as a parameter. But that's not what's in r1 right now. The previous two `ldmia` both clobbered it. At the point of this instruction, the value in r1 is whatever value the jmp_buf had saved for r11.
libc/src/setjmp/arm/longjmp.cpp
nickdesaulniers
@lntue lntue May 24, 2024
ditto
libc/src/setjmp/arm/setjmp.cpp
nickdesaulniers
@lntue lntue May 24, 2024
Is it possible to have (link to) documentations / comments to help newbies like me on these asm?
libc/src/setjmp/arm/longjmp.cpp
nickdesaulniers
@frobtech frobtech May 23, 2024
Extra space before `[` here. This is already a problem for the x86-64 case, but it's not kosher to use plain identifiers here, or in any standard C header. They need to be `__`-prefixed in case a user has `#define opaque syntax-error` before `#include <setjmp.h>`. We should look into a clang-tidy check to enforce that.
Outdated
libc/include/llvm-libc-types/jmp_buf.h
nickdesaulniers
@frobtech frobtech May 23, 2024
The assembler accepts either, but I think we should follow the norm of using `#` for immediates in arm32 and aarch64 assembly.
Outdated
libc/src/setjmp/arm/longjmp.cpp
nickdesaulniers
@frobtech frobtech May 23, 2024
Same issues here: `.w`,`!`, register range syntax, raw string, `#` prefix.
Outdated
libc/src/setjmp/arm/setjmp.cpp
nickdesaulniers
@nickdesaulniers nickdesaulniers May 23, 2024
These warnings (pattern copied from other setjmp impl's in tree) are dumb. You'll get an obvious compiler diagnostic without it pretty quick! I'm going to remove them.
Outdated
libc/src/setjmp/arm/longjmp.cpp
nickdesaulniers