[llvm-branch-commits] [llvm] release/20.x: [SDAG] Ensure load is included in output chain of sincos expansion (#140525) (PR #140703)

2025-05-23 Thread via llvm-branch-commits 

github-actions[bot] wrote:

@MacDue (or anyone else). If you would like to add a note about this fix in the 
release notes (completely optional). Please reply to this comment with a one or 
two sentence description of the fix.  When you are done, please add the 
release:note label to this PR. 

https://github.com/llvm/llvm-project/pull/140703
___
llvm-branch-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] release/20.x: [SDAG] Ensure load is included in output chain of sincos expansion (#140525) (PR #140703)

2025-05-23 Thread Tom Stellard via llvm-branch-commits 

https://github.com/tstellar closed 
https://github.com/llvm/llvm-project/pull/140703
___
llvm-branch-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] release/20.x: [SDAG] Ensure load is included in output chain of sincos expansion (#140525) (PR #140703)

2025-05-23 Thread via llvm-branch-commits 

https://github.com/llvmbot updated 
https://github.com/llvm/llvm-project/pull/140703

>From 9b0832508ede38397c8ebb7a348d50ce1517af4a Mon Sep 17 00:00:00 2001
From: Benjamin Maxwell 
Date: Tue, 20 May 2025 10:43:50 +0100
Subject: [PATCH] [SDAG] Ensure load is included in output chain of sincos
 expansion (#140525)

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 https://github.com/llvm/llvm-project/issues/140491

(cherry picked from commit c9d62491981fe720c1b3255fa2f9ddf744590c65)
---
 .../lib/CodeGen/SelectionDAG/SelectionDAG.cpp |  9 ++-
 .../CodeGen/X86/pr140491-sincos-lifetimes.ll  | 70 +++
 2 files changed, 76 insertions(+), 3 deletions(-)
 create mode 100644 llvm/test/CodeGen/X86/pr140491-sincos-lifetimes.ll

diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp 
b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
index b416c0efbbc4f..eecfb41c2d319 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
@@ -2660,16 +2660,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(ResultPtr)->getIndex());
 }
-SDValue LoadResult =
-getLoad(Node->getValueType(ResNo), DL, CallChain, ResultPtr, PtrInfo);
+
 Results.push_back(LoadResult);
   }
 
diff --git a/llvm/test/CodeGen/X86/pr140491-sincos-lifetimes.ll 
b/llvm/test/CodeGen/X86/pr140491-sincos-lifetimes.ll
new file mode 100644
index 0..2ca99bdc4b316
--- /dev/null
+++ b/llvm/test/CodeGen/X86/pr140491-sincos-lifetimes.ll
@@ -0,0 +1,70 @@
+; RUN: llc < %s | FileCheck %s
+
+; This test is reduced from https://github.com/llvm/llvm-project/issues/140491.
+; It checks that when `@llvm.sincos.f32` is expanded to a call to
+; `sincosf(float, float* out_sin, float* out_cos)` and the store of `%cos` to
+; `%computed` is folded into the `sincosf` call. The use of `%cos`in the later
+; `fneg %cos` -- which expands to a load of `%computed`, will perform the load
+; before the `@llvm.lifetime.end.p0(%computed)` to ensure the correct value is
+; taken for `%cos`.
+
+target triple = "x86_64-sie-ps5"
+
+declare void @use_ptr(ptr readonly)
+
+define i32 @sincos_stack_slot_with_lifetime(float %in)  {
+; 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 $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@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
+; 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: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 $32, %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
+  %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 %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_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.p

[llvm-branch-commits] [llvm] release/20.x: [SDAG] Ensure load is included in output chain of sincos expansion (#140525) (PR #140703)

2025-05-20 Thread Matt Arsenault via llvm-branch-commits 

https://github.com/arsenm approved this pull request.


https://github.com/llvm/llvm-project/pull/140703
___
llvm-branch-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] release/20.x: [SDAG] Ensure load is included in output chain of sincos expansion (#140525) (PR #140703)

2025-05-20 Thread Simon Pilgrim via llvm-branch-commits 

https://github.com/RKSimon approved this pull request.

LGTM

https://github.com/llvm/llvm-project/pull/140703
___
llvm-branch-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] release/20.x: [SDAG] Ensure load is included in output chain of sincos expansion (#140525) (PR #140703)

2025-05-20 Thread Benjamin Maxwell via llvm-branch-commits 

MacDue wrote:

Not sure why the bot is asking me (I think it's fine, but I requested the 
backport). 

cc @arsenm, @RKSimon 
 

https://github.com/llvm/llvm-project/pull/140703
___
llvm-branch-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] release/20.x: [SDAG] Ensure load is included in output chain of sincos expansion (#140525) (PR #140703)

2025-05-20 Thread via llvm-branch-commits 

llvmbot wrote:




@llvm/pr-subscribers-backend-x86

Author: None (llvmbot)


Changes

Backport c9d6249

Requested by: @MacDue

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


2 Files Affected:

- (modified) llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp (+6-3) 
- (added) llvm/test/CodeGen/X86/pr140491-sincos-lifetimes.ll (+70) 


``diff
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp 
b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
index b416c0efbbc4f..eecfb41c2d319 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
@@ -2660,16 +2660,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(ResultPtr)->getIndex());
 }
-SDValue LoadResult =
-getLoad(Node->getValueType(ResNo), DL, CallChain, ResultPtr, PtrInfo);
+
 Results.push_back(LoadResult);
   }
 
diff --git a/llvm/test/CodeGen/X86/pr140491-sincos-lifetimes.ll 
b/llvm/test/CodeGen/X86/pr140491-sincos-lifetimes.ll
new file mode 100644
index 0..2ca99bdc4b316
--- /dev/null
+++ b/llvm/test/CodeGen/X86/pr140491-sincos-lifetimes.ll
@@ -0,0 +1,70 @@
+; RUN: llc < %s | FileCheck %s
+
+; This test is reduced from https://github.com/llvm/llvm-project/issues/140491.
+; It checks that when `@llvm.sincos.f32` is expanded to a call to
+; `sincosf(float, float* out_sin, float* out_cos)` and the store of `%cos` to
+; `%computed` is folded into the `sincosf` call. The use of `%cos`in the later
+; `fneg %cos` -- which expands to a load of `%computed`, will perform the load
+; before the `@llvm.lifetime.end.p0(%computed)` to ensure the correct value is
+; taken for `%cos`.
+
+target triple = "x86_64-sie-ps5"
+
+declare void @use_ptr(ptr readonly)
+
+define i32 @sincos_stack_slot_with_lifetime(float %in)  {
+; 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 $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@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
+; 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: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 $32, %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
+  %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 %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_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)
+  %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
+}
+

``




https://github.com/llvm/llvm-project/pull/140703
___
llvm-branch-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] release/20.x: [SDAG] Ensure load is included in output chain of sincos expansion (#140525) (PR #140703)

2025-05-20 Thread via llvm-branch-commits 

llvmbot wrote:

@MacDue What do you think about merging this PR to the release branch?

https://github.com/llvm/llvm-project/pull/140703
___
llvm-branch-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] release/20.x: [SDAG] Ensure load is included in output chain of sincos expansion (#140525) (PR #140703)

2025-05-20 Thread via llvm-branch-commits 

https://github.com/llvmbot milestoned 
https://github.com/llvm/llvm-project/pull/140703
___
llvm-branch-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] release/20.x: [SDAG] Ensure load is included in output chain of sincos expansion (#140525) (PR #140703)

2025-05-20 Thread via llvm-branch-commits 

https://github.com/llvmbot created 
https://github.com/llvm/llvm-project/pull/140703

Backport c9d6249

Requested by: @MacDue

>From 7d2c28923c503b3e6301ccc038529162f4f33913 Mon Sep 17 00:00:00 2001
From: Benjamin Maxwell 
Date: Tue, 20 May 2025 10:43:50 +0100
Subject: [PATCH] [SDAG] Ensure load is included in output chain of sincos
 expansion (#140525)

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 https://github.com/llvm/llvm-project/issues/140491

(cherry picked from commit c9d62491981fe720c1b3255fa2f9ddf744590c65)
---
 .../lib/CodeGen/SelectionDAG/SelectionDAG.cpp |  9 ++-
 .../CodeGen/X86/pr140491-sincos-lifetimes.ll  | 70 +++
 2 files changed, 76 insertions(+), 3 deletions(-)
 create mode 100644 llvm/test/CodeGen/X86/pr140491-sincos-lifetimes.ll

diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp 
b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
index b416c0efbbc4f..eecfb41c2d319 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
@@ -2660,16 +2660,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(ResultPtr)->getIndex());
 }
-SDValue LoadResult =
-getLoad(Node->getValueType(ResNo), DL, CallChain, ResultPtr, PtrInfo);
+
 Results.push_back(LoadResult);
   }
 
diff --git a/llvm/test/CodeGen/X86/pr140491-sincos-lifetimes.ll 
b/llvm/test/CodeGen/X86/pr140491-sincos-lifetimes.ll
new file mode 100644
index 0..2ca99bdc4b316
--- /dev/null
+++ b/llvm/test/CodeGen/X86/pr140491-sincos-lifetimes.ll
@@ -0,0 +1,70 @@
+; RUN: llc < %s | FileCheck %s
+
+; This test is reduced from https://github.com/llvm/llvm-project/issues/140491.
+; It checks that when `@llvm.sincos.f32` is expanded to a call to
+; `sincosf(float, float* out_sin, float* out_cos)` and the store of `%cos` to
+; `%computed` is folded into the `sincosf` call. The use of `%cos`in the later
+; `fneg %cos` -- which expands to a load of `%computed`, will perform the load
+; before the `@llvm.lifetime.end.p0(%computed)` to ensure the correct value is
+; taken for `%cos`.
+
+target triple = "x86_64-sie-ps5"
+
+declare void @use_ptr(ptr readonly)
+
+define i32 @sincos_stack_slot_with_lifetime(float %in)  {
+; 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 $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@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
+; 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: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 $32, %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
+  %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 %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_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 %compu