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

[WebAssembly] Remove threadlocal.address when disabling TLS #88209

Merged
merged 4 commits into from
Apr 10, 2024

Conversation

MatzeB
Copy link
Contributor

@MatzeB MatzeB commented Apr 9, 2024

Remove llvm.threadlocal.address intrinsic usage when disabling TLS. This fixes errors revealed by the stricter IR verification introduced in PR #87841.

@MatzeB MatzeB marked this pull request as ready for review April 9, 2024 23:36
@llvmbot
Copy link
Collaborator

llvmbot commented Apr 9, 2024

@llvm/pr-subscribers-backend-webassembly

Author: Matthias Braun (MatzeB)

Changes

Remove llvm.threadlocal.address intrinsic usage when disabling TLS. This fixes errors revealed by the stricter IR verification introduced in PR #87841.


Full diff: https://github.com/llvm/llvm-project/pull/88209.diff

2 Files Affected:

  • (modified) llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp (+13)
  • (added) llvm/test/CodeGen/WebAssembly/strip-tls.ll (+28)
diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
index 70685b2e3bb2de..27b0bb2501d4ed 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
@@ -289,6 +289,19 @@ class CoalesceFeaturesAndStripAtomics final : public ModulePass {
 
   bool stripThreadLocals(Module &M) {
     bool Stripped = false;
+    for (auto &F : M) {
+      for (auto &B : F) {
+        for (auto &I : make_early_inc_range(B)) {
+          if (IntrinsicInst *II = dyn_cast<IntrinsicInst>(&I)) {
+            if (II->getIntrinsicID() == Intrinsic::threadlocal_address) {
+              II->replaceAllUsesWith(II->getArgOperand(0));
+              II->eraseFromParent();
+              Stripped = true;
+            }
+          }
+        }
+      }
+    }
     for (auto &GV : M.globals()) {
       if (GV.isThreadLocal()) {
         Stripped = true;
diff --git a/llvm/test/CodeGen/WebAssembly/strip-tls.ll b/llvm/test/CodeGen/WebAssembly/strip-tls.ll
new file mode 100644
index 00000000000000..281fe0aee7a497
--- /dev/null
+++ b/llvm/test/CodeGen/WebAssembly/strip-tls.ll
@@ -0,0 +1,28 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 4
+; RUN: llc -o - %s -mattr=-bulk-memory | FileCheck --check-prefix=NOTLS %s
+; RUN: llc -o - %s -mattr=+bulk-memory,+atomics | FileCheck --check-prefix=TLS %s
+
+target triple = "wasm32--"
+
+@tls = internal thread_local global i32 0
+
+define i32 @func() {
+; NOTLS-LABEL: func:
+; NOTLS:         .functype func () -> (i32)
+; NOTLS-NEXT:  # %bb.0:
+; NOTLS-NEXT:    i32.const 0
+; NOTLS-NEXT:    i32.load tls
+; NOTLS-NEXT:    # fallthrough-return
+;
+; TLS-LABEL: func:
+; TLS:         .functype func () -> (i32)
+; TLS-NEXT:  # %bb.0:
+; TLS-NEXT:    global.get __tls_base
+; TLS-NEXT:    i32.const tls@TLSREL
+; TLS-NEXT:    i32.add
+; TLS-NEXT:    i32.load 0
+; TLS-NEXT:    # fallthrough-return
+  %p = call ptr @llvm.threadlocal.address.p0(ptr @tls)
+  %v = load i32, ptr %p
+  ret i32 %v
+}

@MatzeB MatzeB changed the title WebAssembly: Remove threadlocal_address intrinsics when disabling TLS WebAssembly: Remove threadlocal.address when disabling TLS Apr 9, 2024
@MatzeB MatzeB changed the title WebAssembly: Remove threadlocal.address when disabling TLS [WebAssembly] Remove threadlocal.address when disabling TLS Apr 9, 2024
Copy link
Member

@dschuff dschuff left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this!

@MatzeB MatzeB force-pushed the webasm_threadlocal_address_fix branch from bf5326d to e4737aa Compare April 10, 2024 18:03
@MatzeB MatzeB merged commit acb7ddc into llvm:main Apr 10, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants