[llvm] [SDAG] Ensure load is included in output chain of sincos expansion (PR #140525)
Benjamin Maxwell via llvm-commits
llvm-commits at lists.llvm.org
Mon May 19 05:02:57 PDT 2025
https://github.com/MacDue updated https://github.com/llvm/llvm-project/pull/140525
>From aa943b1a2ce6ef887f640021e9126b0240196119 Mon Sep 17 00:00:00 2001
From: Benjamin Maxwell <benjamin.maxwell at arm.com>
Date: Mon, 19 May 2025 10:14:55 +0000
Subject: [PATCH 1/4] Precommit test
---
.../X86/sincos-lifetimes-issue-140491.ll | 61 +++++++++++++++++++
llvm/utils/UpdateTestChecks/asm.py | 6 +-
2 files changed, 64 insertions(+), 3 deletions(-)
create mode 100644 llvm/test/CodeGen/X86/sincos-lifetimes-issue-140491.ll
diff --git a/llvm/test/CodeGen/X86/sincos-lifetimes-issue-140491.ll b/llvm/test/CodeGen/X86/sincos-lifetimes-issue-140491.ll
new file mode 100644
index 0000000000000..5e7e082910b83
--- /dev/null
+++ b/llvm/test/CodeGen/X86/sincos-lifetimes-issue-140491.ll
@@ -0,0 +1,61 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --no_x86_scrub_sp --no_x86_scrub_mem_shuffle --version 5
+; RUN: llc < %s | FileCheck %s
+
+target triple = "x86_64-sie-ps5"
+
+declare hidden void @use_ptr(ptr readonly) local_unnamed_addr
+
+define hidden noundef i32 @sincos_stack_slot_with_lifetime(float %in) local_unnamed_addr {
+; CHECK-LABEL: sincos_stack_slot_with_lifetime:
+; CHECK: # %bb.0: # %entry
+; CHECK-NEXT: pushq %rbx
+; CHECK-NEXT: .cfi_def_cfa_offset 16
+; CHECK-NEXT: subq $16, %rsp
+; CHECK-NEXT: .cfi_def_cfa_offset 32
+; CHECK-NEXT: .cfi_offset %rbx, -16
+; CHECK-NEXT: leaq 12(%rsp), %rdi
+; CHECK-NEXT: leaq 8(%rsp), %rbx
+; CHECK-NEXT: movq %rbx, %rsi
+; CHECK-NEXT: callq sincosf at PLT
+; CHECK-NEXT: movq %rbx, %rdi
+; CHECK-NEXT: callq use_ptr
+; CHECK-NEXT: movss 12(%rsp), %xmm0 # xmm0 = mem[0],zero,zero,zero
+; CHECK-NEXT: xorps {{\.?LCPI[0-9]+_[0-9]+}}(%rip), %xmm0
+; CHECK-NEXT: movss %xmm0, 8(%rsp)
+; CHECK-NEXT: leaq 8(%rsp), %rdi
+; CHECK-NEXT: callq use_ptr
+; CHECK-NEXT: movss 8(%rsp), %xmm0 # xmm0 = mem[0],zero,zero,zero
+; CHECK-NEXT: xorps {{\.?LCPI[0-9]+_[0-9]+}}(%rip), %xmm0
+; CHECK-NEXT: movss %xmm0, 8(%rsp)
+; CHECK-NEXT: leaq 8(%rsp), %rdi
+; CHECK-NEXT: callq use_ptr
+; CHECK-NEXT: xorl %eax, %eax
+; CHECK-NEXT: addq $16, %rsp
+; CHECK-NEXT: .cfi_def_cfa_offset 16
+; CHECK-NEXT: popq %rbx
+; CHECK-NEXT: .cfi_def_cfa_offset 8
+; CHECK-NEXT: retq
+entry:
+ %computed = alloca float, align 4
+ %computed1 = alloca float, align 4
+ %computed3 = alloca float, align 4
+ %0 = tail call { float, float } @llvm.sincos.f32(float %in)
+ %1 = extractvalue { float, float } %0, 0
+ %2 = extractvalue { float, float } %0, 1
+ call void @llvm.lifetime.start.p0(i64 4, ptr nonnull %computed)
+ store float %2, ptr %computed, align 4
+ call void @use_ptr(ptr nonnull %computed)
+ call void @llvm.lifetime.end.p0(i64 4, ptr nonnull %computed)
+ call void @llvm.lifetime.start.p0(i64 4, ptr nonnull %computed1)
+ %fneg = fneg float %1
+ store float %fneg, ptr %computed1, align 4
+ call void @use_ptr(ptr nonnull %computed1)
+ call void @llvm.lifetime.end.p0(i64 4, ptr nonnull %computed1)
+ call void @llvm.lifetime.start.p0(i64 4, ptr nonnull %computed3)
+ %fneg4 = fneg float %2
+ store float %fneg4, ptr %computed3, align 4
+ call void @use_ptr(ptr nonnull %computed3)
+ call void @llvm.lifetime.end.p0(i64 4, ptr nonnull %computed3)
+ ret i32 0
+}
+
diff --git a/llvm/utils/UpdateTestChecks/asm.py b/llvm/utils/UpdateTestChecks/asm.py
index 7d4fb7d8e1504..e7097fdf7c9d1 100644
--- a/llvm/utils/UpdateTestChecks/asm.py
+++ b/llvm/utils/UpdateTestChecks/asm.py
@@ -294,10 +294,10 @@ def scrub_asm_x86(asm, args):
else:
asm = SCRUB_X86_SHUFFLES_RE.sub(r"\1 {{.*#+}} \2", asm)
- # Detect stack spills and reloads and hide their exact offset and whether
- # they used the stack pointer or frame pointer.
- asm = SCRUB_X86_SPILL_RELOAD_RE.sub(r"{{[-0-9]+}}(%\1{{[sb]}}p)\2", asm)
if getattr(args, "x86_scrub_sp", True):
+ # Detect stack spills and reloads and hide their exact offset and whether
+ # they used the stack pointer or frame pointer.
+ asm = SCRUB_X86_SPILL_RELOAD_RE.sub(r"{{[-0-9]+}}(%\1{{[sb]}}p)\2", asm)
# Generically match the stack offset of a memory operand.
asm = SCRUB_X86_SP_RE.sub(r"{{[0-9]+}}(%\1)", asm)
if getattr(args, "x86_scrub_rip", False):
>From c0042c0e54d9f6d2c01b82a7f6fdacfd974bf048 Mon Sep 17 00:00:00 2001
From: Benjamin Maxwell <benjamin.maxwell at arm.com>
Date: Mon, 19 May 2025 10:17:30 +0000
Subject: [PATCH 2/4] [SDAG] Ensure load is included in output chain of sincos
expansion
The load not being included in the chain meant that it could materialize
after a `@llvm.lifetime.end` annotation on the pointer. This could
result in miscompiles if the stack slot is reused for another value.
Fixes #140491
---
llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp | 9 ++++++---
llvm/test/CodeGen/X86/sincos-lifetimes-issue-140491.ll | 10 ++++++----
2 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
index 5d640c39a56d5..304ceb53f8793 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
@@ -2625,16 +2625,19 @@ bool SelectionDAG::expandMultipleResultFPLibCall(
continue;
}
MachinePointerInfo PtrInfo;
+ SDValue LoadResult =
+ getLoad(Node->getValueType(ResNo), DL, CallChain, ResultPtr, PtrInfo);
+ SDValue OutChain = LoadResult.getValue(1);
+
if (StoreSDNode *ST = ResultStores[ResNo]) {
// Replace store with the library call.
- ReplaceAllUsesOfValueWith(SDValue(ST, 0), CallChain);
+ ReplaceAllUsesOfValueWith(SDValue(ST, 0), OutChain);
PtrInfo = ST->getPointerInfo();
} else {
PtrInfo = MachinePointerInfo::getFixedStack(
getMachineFunction(), cast<FrameIndexSDNode>(ResultPtr)->getIndex());
}
- SDValue LoadResult =
- getLoad(Node->getValueType(ResNo), DL, CallChain, ResultPtr, PtrInfo);
+
Results.push_back(LoadResult);
}
diff --git a/llvm/test/CodeGen/X86/sincos-lifetimes-issue-140491.ll b/llvm/test/CodeGen/X86/sincos-lifetimes-issue-140491.ll
index 5e7e082910b83..bb42f176987e2 100644
--- a/llvm/test/CodeGen/X86/sincos-lifetimes-issue-140491.ll
+++ b/llvm/test/CodeGen/X86/sincos-lifetimes-issue-140491.ll
@@ -10,13 +10,15 @@ define hidden noundef i32 @sincos_stack_slot_with_lifetime(float %in) local_unna
; CHECK: # %bb.0: # %entry
; CHECK-NEXT: pushq %rbx
; CHECK-NEXT: .cfi_def_cfa_offset 16
-; CHECK-NEXT: subq $16, %rsp
-; CHECK-NEXT: .cfi_def_cfa_offset 32
+; CHECK-NEXT: subq $32, %rsp
+; CHECK-NEXT: .cfi_def_cfa_offset 48
; CHECK-NEXT: .cfi_offset %rbx, -16
; CHECK-NEXT: leaq 12(%rsp), %rdi
; CHECK-NEXT: leaq 8(%rsp), %rbx
; CHECK-NEXT: movq %rbx, %rsi
; CHECK-NEXT: callq sincosf at PLT
+; CHECK-NEXT: movss 8(%rsp), %xmm0 # xmm0 = mem[0],zero,zero,zero
+; CHECK-NEXT: movaps %xmm0, 16(%rsp) # 16-byte Spill
; CHECK-NEXT: movq %rbx, %rdi
; CHECK-NEXT: callq use_ptr
; CHECK-NEXT: movss 12(%rsp), %xmm0 # xmm0 = mem[0],zero,zero,zero
@@ -24,13 +26,13 @@ define hidden noundef i32 @sincos_stack_slot_with_lifetime(float %in) local_unna
; CHECK-NEXT: movss %xmm0, 8(%rsp)
; CHECK-NEXT: leaq 8(%rsp), %rdi
; CHECK-NEXT: callq use_ptr
-; CHECK-NEXT: movss 8(%rsp), %xmm0 # xmm0 = mem[0],zero,zero,zero
+; CHECK-NEXT: movaps 16(%rsp), %xmm0 # 16-byte Reload
; CHECK-NEXT: xorps {{\.?LCPI[0-9]+_[0-9]+}}(%rip), %xmm0
; CHECK-NEXT: movss %xmm0, 8(%rsp)
; CHECK-NEXT: leaq 8(%rsp), %rdi
; CHECK-NEXT: callq use_ptr
; CHECK-NEXT: xorl %eax, %eax
-; CHECK-NEXT: addq $16, %rsp
+; CHECK-NEXT: addq $32, %rsp
; CHECK-NEXT: .cfi_def_cfa_offset 16
; CHECK-NEXT: popq %rbx
; CHECK-NEXT: .cfi_def_cfa_offset 8
>From f24778051ec1f3153955e5a5200be03f34e35e29 Mon Sep 17 00:00:00 2001
From: Benjamin Maxwell <benjamin.maxwell at arm.com>
Date: Mon, 19 May 2025 10:52:56 +0000
Subject: [PATCH 3/4] Fixups
---
.../X86/sincos-lifetimes-issue-140491.ll | 21 +++++++++----------
llvm/utils/UpdateTestChecks/asm.py | 6 +++---
2 files changed, 13 insertions(+), 14 deletions(-)
diff --git a/llvm/test/CodeGen/X86/sincos-lifetimes-issue-140491.ll b/llvm/test/CodeGen/X86/sincos-lifetimes-issue-140491.ll
index bb42f176987e2..5a6f607de69ee 100644
--- a/llvm/test/CodeGen/X86/sincos-lifetimes-issue-140491.ll
+++ b/llvm/test/CodeGen/X86/sincos-lifetimes-issue-140491.ll
@@ -1,11 +1,10 @@
-; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --no_x86_scrub_sp --no_x86_scrub_mem_shuffle --version 5
; RUN: llc < %s | FileCheck %s
target triple = "x86_64-sie-ps5"
-declare hidden void @use_ptr(ptr readonly) local_unnamed_addr
+declare void @use_ptr(ptr readonly)
-define hidden noundef i32 @sincos_stack_slot_with_lifetime(float %in) local_unnamed_addr {
+define i32 @sincos_stack_slot_with_lifetime(float %in) {
; CHECK-LABEL: sincos_stack_slot_with_lifetime:
; CHECK: # %bb.0: # %entry
; CHECK-NEXT: pushq %rbx
@@ -41,21 +40,21 @@ entry:
%computed = alloca float, align 4
%computed1 = alloca float, align 4
%computed3 = alloca float, align 4
- %0 = tail call { float, float } @llvm.sincos.f32(float %in)
- %1 = extractvalue { float, float } %0, 0
- %2 = extractvalue { float, float } %0, 1
+ %sincos = tail call { float, float } @llvm.sincos.f32(float %in)
+ %sin = extractvalue { float, float } %sincos, 0
+ %cos = extractvalue { float, float } %sincos, 1
call void @llvm.lifetime.start.p0(i64 4, ptr nonnull %computed)
- store float %2, ptr %computed, align 4
+ store float %cos, ptr %computed, align 4
call void @use_ptr(ptr nonnull %computed)
call void @llvm.lifetime.end.p0(i64 4, ptr nonnull %computed)
call void @llvm.lifetime.start.p0(i64 4, ptr nonnull %computed1)
- %fneg = fneg float %1
- store float %fneg, ptr %computed1, align 4
+ %fneg_sin = fneg float %sin
+ store float %fneg_sin, ptr %computed1, align 4
call void @use_ptr(ptr nonnull %computed1)
call void @llvm.lifetime.end.p0(i64 4, ptr nonnull %computed1)
call void @llvm.lifetime.start.p0(i64 4, ptr nonnull %computed3)
- %fneg4 = fneg float %2
- store float %fneg4, ptr %computed3, align 4
+ %fneg_cos = fneg float %cos
+ store float %fneg_cos, ptr %computed3, align 4
call void @use_ptr(ptr nonnull %computed3)
call void @llvm.lifetime.end.p0(i64 4, ptr nonnull %computed3)
ret i32 0
diff --git a/llvm/utils/UpdateTestChecks/asm.py b/llvm/utils/UpdateTestChecks/asm.py
index e7097fdf7c9d1..7d4fb7d8e1504 100644
--- a/llvm/utils/UpdateTestChecks/asm.py
+++ b/llvm/utils/UpdateTestChecks/asm.py
@@ -294,10 +294,10 @@ def scrub_asm_x86(asm, args):
else:
asm = SCRUB_X86_SHUFFLES_RE.sub(r"\1 {{.*#+}} \2", asm)
+ # Detect stack spills and reloads and hide their exact offset and whether
+ # they used the stack pointer or frame pointer.
+ asm = SCRUB_X86_SPILL_RELOAD_RE.sub(r"{{[-0-9]+}}(%\1{{[sb]}}p)\2", asm)
if getattr(args, "x86_scrub_sp", True):
- # Detect stack spills and reloads and hide their exact offset and whether
- # they used the stack pointer or frame pointer.
- asm = SCRUB_X86_SPILL_RELOAD_RE.sub(r"{{[-0-9]+}}(%\1{{[sb]}}p)\2", asm)
# Generically match the stack offset of a memory operand.
asm = SCRUB_X86_SP_RE.sub(r"{{[0-9]+}}(%\1)", asm)
if getattr(args, "x86_scrub_rip", False):
>From af2b006a9ec3b6db3f9ac9043eddc924009693d5 Mon Sep 17 00:00:00 2001
From: Benjamin Maxwell <benjamin.maxwell at arm.com>
Date: Mon, 19 May 2025 12:02:46 +0000
Subject: [PATCH 4/4] Rename test
---
...cos-lifetimes-issue-140491.ll => pr140491-sincos-lifetimes.ll} | 0
1 file changed, 0 insertions(+), 0 deletions(-)
rename llvm/test/CodeGen/X86/{sincos-lifetimes-issue-140491.ll => pr140491-sincos-lifetimes.ll} (100%)
diff --git a/llvm/test/CodeGen/X86/sincos-lifetimes-issue-140491.ll b/llvm/test/CodeGen/X86/pr140491-sincos-lifetimes.ll
similarity index 100%
rename from llvm/test/CodeGen/X86/sincos-lifetimes-issue-140491.ll
rename to llvm/test/CodeGen/X86/pr140491-sincos-lifetimes.ll
More information about the llvm-commits
mailing list