Skip to content

Conversation

dschuff
Copy link
Member

@dschuff dschuff commented Aug 29, 2025

WebAssemblyRegStackfy checks for writes to the stack pointer to avoid
stackifying across them, but it wasn't prepared for other global_set
instructions (such as writes in addrspace 1).

Fixes #156055

Thanks to @QuantumSegfault for reporting and identifying the offending code.

WebAssemblyRegStackfy checks for writes to the stack pointer to avoid
stackifying across them, but it wasn't prepared for other global_set
instructions (such as writes in addrspace 1).

Fixes llvm#156055
@llvmbot
Copy link
Member

llvmbot commented Aug 29, 2025

@llvm/pr-subscribers-backend-webassembly

Author: Derek Schuff (dschuff)

Changes

WebAssemblyRegStackfy checks for writes to the stack pointer to avoid
stackifying across them, but it wasn't prepared for other global_set
instructions (such as writes in addrspace 1).

Fixes #156055


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

2 Files Affected:

  • (modified) llvm/lib/Target/WebAssembly/WebAssemblyRegStackify.cpp (+2-1)
  • (modified) llvm/test/CodeGen/WebAssembly/global-set.ll (+15)
diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyRegStackify.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyRegStackify.cpp
index bc91c6424b63e..08ca20b5eef6e 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyRegStackify.cpp
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyRegStackify.cpp
@@ -247,7 +247,8 @@ static void query(const MachineInstr &MI, bool &Read, bool &Write,
   // Check for writes to __stack_pointer global.
   if ((MI.getOpcode() == WebAssembly::GLOBAL_SET_I32 ||
        MI.getOpcode() == WebAssembly::GLOBAL_SET_I64) &&
-      strcmp(MI.getOperand(0).getSymbolName(), "__stack_pointer") == 0)
+      MI.getOperand(0).isSymbol() &&
+      !strcmp(MI.getOperand(0).getSymbolName(), "__stack_pointer"))
     StackPointer = true;
 
   // Analyze calls.
diff --git a/llvm/test/CodeGen/WebAssembly/global-set.ll b/llvm/test/CodeGen/WebAssembly/global-set.ll
index 7db374528fe9b..f76006c2863b2 100644
--- a/llvm/test/CodeGen/WebAssembly/global-set.ll
+++ b/llvm/test/CodeGen/WebAssembly/global-set.ll
@@ -45,6 +45,21 @@ define void @set_f64_global(double %v) {
   ret void
 }
 
+declare i32 @get_i32()
+define i32 @testFunc() {
+; CHECK-LABEL: testFunc:
+; CHECK-NEXT: .functype
+; CHECK-NEXT: .local
+; CHECK-NEXT: call get_i32
+; CHECK-NEXT: local.tee
+; CHECK-NEXT: global.set i32_global
+; CHECK-NEXT: local.get
+; CHECK-NEXT: end_function
+  %1 = call i32 @get_i32()
+  store i32 %1, ptr addrspace(1) @i32_global
+  ret i32 %1
+}
+
 ; CHECK: .globaltype i32_global, i32
 ; CHECK: .globl i32_global
 ; CHECK-LABEL: i32_global:

Copy link
Member

@aheejin aheejin left a comment

Choose a reason for hiding this comment

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

LGTM % the test comment

@dschuff dschuff enabled auto-merge (squash) September 2, 2025 20:14
Copy link

@QuantumSegfault QuantumSegfault left a comment

Choose a reason for hiding this comment

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

This fix works for me.

I considered opening a PR myself, but wasn't sure the problem/fix was as simple as it seemed (and is).

@dschuff
Copy link
Member Author

dschuff commented Sep 2, 2025

Great, yeah thanks for the report and the reproducer, it made it easy :)

@dschuff dschuff disabled auto-merge September 2, 2025 23:21
@dschuff dschuff merged commit a3c41dd into llvm:main Sep 2, 2025
7 of 8 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.

[WebAssembly] Assertion failed: (isSymbol() && "Wrong MachineOperand accessor")
4 participants