Skip to content

Enable thread_local for OpenHarmony#158750

Open
12101111 wants to merge 2 commits into
rust-lang:mainfrom
12101111:ohos-emutls
Open

Enable thread_local for OpenHarmony#158750
12101111 wants to merge 2 commits into
rust-lang:mainfrom
12101111:ohos-emutls

Conversation

@12101111

@12101111 12101111 commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

I have run ui test on OpenHarmony 6.1.1 (HarmonyOS 6.1.0.130 SP15) and no regression was found.

A small performance test shows some improvement:

const ITERATIONS: usize = 10_000_000;

thread_local! {
    static TLS_CELL: Cell<usize> = Cell::new(0);
}

fn main() {
    for _ in 0..ITERATIONS {
        TLS_CELL.with(|cell| {
            let v = cell.get();
            cell.set(v + 1);
            black_box(v);
        });
    }
}
  • Old: 407.5ms
  • New: 366.5ms

This also fix potential fatal runtime error: out of TLS keys, aborting issues

@rustbot

rustbot commented Jul 3, 2026

Copy link
Copy Markdown
Collaborator

These commits modify compiler targets.
(See the Target Tier Policy.)

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 3, 2026
@rustbot

rustbot commented Jul 3, 2026

Copy link
Copy Markdown
Collaborator

r? @oli-obk

rustbot has assigned @oli-obk.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: codegen, compiler
  • codegen, compiler expanded to 75 candidates
  • Random selection from 22 candidates

@Urgau

Urgau commented Jul 3, 2026

Copy link
Copy Markdown
Member

cc @Amanieu @cceerczw (as the target maintainers for OpenHarmony)

workingjubilee
workingjubilee reviewed Jul 3, 2026
View reviewed changes
Comment thread compiler/rustc_codegen_ssa/src/back/linker.rs
Comment on lines +880 to +890
// Cygwin don't need this as PE/Coff don't have weak symbols
if self.sess.target.tls_model == TlsModel::Emulated
&& self.sess.target.binary_format == BinaryFormat::Elf
{
let has_tls = symbols.iter().any(|s| s.kind == SymbolExportKind::Tls);
if has_tls {
// use `*` wildcard to avoid error with `--no-undefined-version`
debug!(" __emutls_get_address*;");
writeln!(f, " __emutls_get_address*;")?;
}
}

@workingjubilee workingjubilee Jul 3, 2026 •
edited by rustbot
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this necessary?

There's several other Rust targets that are TlsModel::Emulated && BinaryFormat::Elf and they have been working... uh, I can't say perfectly, but the problems we have with them not working aren't caused by this symbol being non-exported, as far as I know.

View changes since the review

@12101111 12101111 Jul 4, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tls-dylib-access.rs will fail without this symbol being exported (at least on Android and OHOS, not sure about OpenBSD): #147590 (comment)

workingjubilee
workingjubilee reviewed Jul 3, 2026
View reviewed changes

@workingjubilee workingjubilee left a comment •
edited by rustbot
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OHOS is a Linux-based target, is there a reason it can't use TlsModel::GeneralDynamic like other Linux targets do?

View changes since this review

@Amanieu

Amanieu commented Jul 4, 2026

Copy link
Copy Markdown
Member

ELF TLS should work fine, the only concern I have is ABI compatibility with extern "C" { #[thread_local] static ... } which accesses a TLS variable via FFI. LLVM explicitly specifies that emulated TLS is enabled by default on OHOS targets:

  /// Tests whether the target uses emulated TLS as default.
  ///
  /// Note: Android API level 29 (10) introduced ELF TLS.
  bool hasDefaultEmulatedTLS() const {
    return (isAndroid() && isAndroidVersionLT(29)) || isOSOpenBSD() ||
           isWindowsCygwinEnvironment() || isOHOSFamily();
  }

@workingjubilee

workingjubilee commented Jul 6, 2026

Copy link
Copy Markdown
Member

Hm. Do we have any tests that would cover interactions with such things across the FFI boundary?

@12101111

12101111 commented Jul 6, 2026

Copy link
Copy Markdown
Contributor Author

The only test that conver TLS FFI is https://github.com/rust-lang/rust/blob/main/tests/run-make/raw-dylib-elf/main.rs

@oli-obk oli-obk assigned Amanieu and unassigned oli-obk Jul 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@workingjubilee workingjubilee workingjubilee left review comments

At least 1 approving review is required to merge this pull request.

Assignees

@Amanieu Amanieu

Labels

S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@12101111 @rustbot @Urgau @Amanieu @workingjubilee @oli-obk