-
Notifications
You must be signed in to change notification settings - Fork 15k
[WebAssembly] Guard use of getSymbolName with isSymbol #156105
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
Conversation
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
@llvm/pr-subscribers-backend-webassembly Author: Derek Schuff (dschuff) ChangesWebAssemblyRegStackfy checks for writes to the stack pointer to avoid Fixes #156055 Full diff: https://github.com/llvm/llvm-project/pull/156105.diff 2 Files Affected:
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:
|
There was a problem hiding this 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
There was a problem hiding this 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).
Great, yeah thanks for the report and the reproducer, it made it easy :) |
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.