search

rust3ds / pthread-3ds

PThread implementation for Nintendo 3DS Horizon OS targets. Keep in mind that Horizon OS uses a cooperative, and not preemptive, threading model.
Apache License 2.0
12 stars 7 forks source link

Discuss the removal of the TLS code #23

Closed Meziu closed 1 year ago

Meziu commented 1 year ago

Having a fallback is a nice thing, but for how slow it is (and the fact it's not even fully working) I doubt anyone should use it. @rust3ds/active Do you think we should remove it?

AzureMarker commented 1 year ago

Is there an alternative? Don't we need it implemented?

Meziu commented 1 year ago

Since the latest changes in rustc, there are now no issues regarding linking. Also, @ian-h-chamberlain pushed the changes to libctru to fix the TLS alignment. Right now thread_local! uses the built-in TLS via the #[thread_local] flag, and not our implementation. That also calls the destructors on the TLS vars on drop.

ian-h-chamberlain commented 1 year ago

I'm not 100% convinced we're totally done here, see my comment in https://github.com/rust3ds/pthread-3ds/issues/19#issuecomment-1473959126

As a test, I also just commented out some of the pthread_key_* functions and I did in fact see some link errors on rustc 1.70.0-nightly (511364e78 2023-03-16):

  = note: /opt/devkitpro/devkitARM/bin/../lib/gcc/arm-none-eabi/12.2.0/../../../../arm-none-eabi/bin/ld: /home/deck/Documents/3ds/ctru-rs/target/armv6k-nintendo-3ds/debug/deps/libstd-c55a323fa3aa843b.rlib(std-c55a323fa3aa843b.std.1a24a1b1-cgu.2.rcgu.o): in function `std::sys::unix::thread_local_key::create':
          /home/deck/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/sys/unix/thread_local_key.rs:10: undefined reference to `pthread_key_create'
          /opt/devkitpro/devkitARM/bin/../lib/gcc/arm-none-eabi/12.2.0/../../../../arm-none-eabi/bin/ld: /home/deck/Documents/3ds/ctru-rs/target/armv6k-nintendo-3ds/debug/deps/libstd-c55a323fa3aa843b.rlib(std-c55a323fa3aa843b.std.1a24a1b1-cgu.2.rcgu.o): in function `std::sys::unix::thread_local_key::set':
          /home/deck/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/sys/unix/thread_local_key.rs:16: undefined reference to `pthread_setspecific'
          /opt/devkitpro/devkitARM/bin/../lib/gcc/arm-none-eabi/12.2.0/../../../../arm-none-eabi/bin/ld: /home/deck/Documents/3ds/ctru-rs/target/armv6k-nintendo-3ds/debug/deps/libstd-c55a323fa3aa843b.rlib(std-c55a323fa3aa843b.std.1a24a1b1-cgu.2.rcgu.o): in function `std::sys::unix::thread_local_key::get':
          /home/deck/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/sys/unix/thread_local_key.rs:22: undefined reference to `pthread_getspecific'
          collect2: error: ld returned 1 exit status

  = note: some `extern` functions couldn't be found; some native libraries may need to be installed or have their path specified
  = note: use the `-l` flag to specify native libraries to link
  = note: use the `cargo:rustc-link-lib` directive to specify the native libraries to link with Cargo (see https://doc.rust-lang.org/cargo/reference/build-scripts.html#cargorustc-link-libkindname)

error: could not compile `ctru-rs` (example "thread-locals") due to previous error

Maybe we are still missing some #[cfg] somewhere upstream to get it to use the fast_local implementation?

One other consideration: I think we should focus on supporting std for the most part, but if there are any other libraries that expect to be able to use normal pthread APIs, it might still be nice to have these extern symbols defined. I don't know of any such libraries offhand, so maybe we shouldn't worry about it now, but just something that occurred to me.

Meziu commented 1 year ago

I have changed my mind on this issue, especially thinking about the newest developments with the upstream pthread.