Implement __sync builtins for thumbv6-none-eabi

[taiki-e]

This is a PR for thumbv6-none-eabi (bere-metal Armv6k in Thumb mode) which proposed to be added by rust-lang/rust#150138.

Armv6k supports atomic instructions, but they are unavailable in Thumb mode unless Thumb-2 instructions available (v6t2).

Using Thumb interworking (can be used via #[instruction_set]) allows us to use these instructions even from Thumb mode without Thumb-2 instructions, but LLVM does not implement that processing (as of LLVM 21), so this PR implements it in compiler-builtins.

The code around __sync builtins is basically copied from arm_linux.rs which uses kernel_user_helpers for atomic implementation.
The atomic implementation is a port of my atomic-maybe-uninit inline assembly code.

This PR has been tested on QEMU 10.2.0 using patched compiler-builtins and core that applied the changes in this PR and rust-lang/rust#150138 and the portable-atomic no-std test suite (can be run with ./tools/no-std.sh thumbv6-none-eabi on that repo) which tests wrappers around core::sync::atomic. (Note that the target-spec used in test sets max-atomic-width to 32 and atomic_cas to true, unlike the current rust-lang/rust#150138.) The original atomic-maybe-uninit implementation has been tested on real Arm hardware.

(Note that Armv6k also supports 64-bit atomic instructions, but they are skipped here. This is because there is no corresponding code in arm_linux.rs (since the kernel requirements increased in 1.64, it may be possible to implement 64-bit atomics there as well. see also taiki-e/portable-atomic#82), the code becomes more complex than for 32-bit and smaller atomics.)

cc @thejpster (target maintainer)

I'll undraft this PR once the target maintainer approved this approach.

[thejpster]

This looks OK to me, but it's a bit beyond my Arm experience.

[tgross35]

Looks pretty good to me, thanks for the patch. This should wait until one of the compiler leads approves the target PR, though.

[tgross35]

Nit: could you name this arm_thumb_sync_builtins or similar?

[tgross35]

Actually, I think we may as well group this by functionality like we have for other areas, something like:

src/sync/
    arm_linux.rs
    thumbv6k.rs
    arm_thumb_shared.rs // this file

I'll move the aarch64 builtins there too.

[taiki-e]

Done in 5ccc285.

[tgross35]

For my own understanding: why is the barrier needed between the first ldrex/strex, but not between subsequent l/s pairs?

[taiki-e]

This (omit the preceding fence if no write operation is performed) is what LLVM does (https://godbolt.org/z/G7jGEGz6q), but I can rewrite to always emits two fences.

[tgross35]

I don't know enough about the target to know what is better here, so it's your call. Either way, mind adding a comment?

[taiki-e]

Added a commit in 2a6a68c.

[tgross35]

This PR has been tested on QEMU 10.2.0 using patched compiler-builtins and core that applied the changes in this PR and rust-lang/rust#150138 and the portable-atomic no-std test suite (can be run with ./tools/no-std.sh thumbv6-none-eabi on that repo) which tests wrappers around core::sync::atomic.

If you get the chance at some point, I would be rather interested in having some of your atomic test suite here. Especially the part you mention since we don't have any no-std tests set up.

(Note that Armv6k also supports 64-bit atomic instructions, but they are skipped here. This is because there is no corresponding code in arm_linux.rs (since the kernel requirements increased in 1.64, it may be possible to implement 64-bit atomics there as well. see also taiki-e/portable-atomic#82), the code becomes more complex than for 32-bit and smaller atomics.)

Assuming this would allow us to set target_has_atomic = "64", I think that would be worth it. Just not pressing, of course.

[thejpster]

@davidtwco said “LGTM” on the target but the target is blocked waiting for this PR. But it seems this PR is blocked waiting for the target?

@taiki-e - should we still do this one first?

[taiki-e]

@thejpster

But it seems this PR is blocked waiting for the target?

This PR is fine without the target. (See the CI change using a custom target.)

(However, the target or patched core is required to run the test suite not included in this PR.)

[tgross35]

Requested one more comment in #1050 (comment) but otherwise this looks great to me, thank you for the detailed work.

I'd still mildly prefer if rust-lang/rust#150138 merges first so this PR can test against the target as-added without the back-and-forth. But if a few days go by without any movement there, I'll go ahead and apply this.

[taiki-e]

I'd still mildly prefer if rust-lang/rust#150138 merges first so this PR can test against the target as-added without the back-and-forth. But if a few days go by without any movement there, I'll go ahead and apply this.

I think it is easier to understand/use if the target consistently provides atomics regardless of the compiler version from the user's point of view. (In cases where atomic operations are missing, it may be necessary to enable the specific optional feature to enable atomic.)

(Also, to run tests, it's preferable that atomics are enabled for the target (manually calling the sync built-in is annoying), so I guess adding no-std tests will need multiple PRs on the compiler-builtins side anyway.)

[tgross35]

Fair enough, applied now. I probably won't do a sync until after #1061 (unless that takes a long time) since that may have some UB in atomic fixes. (Bit of a weird issue, any idea if it's the sort of thing your testsuite would have caught?)