Skip to content

release/20.x: [X86][TargetLowering] Avoid deleting temporary nodes in getNegatedExpression (#139029) #139356

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

Open
wants to merge 1 commit into
base: release/20.x
Choose a base branch
from

Conversation

llvmbot
Copy link
Member

@llvmbot llvmbot commented May 10, 2025

Backport 143cce7

Requested by: @dtcxzyw

…pression` (llvm#139029)

In the original case, the third call to `getCheaperNegatedExpression`
deletes the SDNode returned by the first call.
Similar to 74e6030, this patch uses
`HandleSDNodes` to prevent nodes from being deleted by subsequent calls.
Closes llvm#138944.

(cherry picked from commit 143cce7)
@llvmbot
Copy link
Member Author

llvmbot commented May 10, 2025

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

@llvmbot
Copy link
Member Author

llvmbot commented May 10, 2025

@llvm/pr-subscribers-backend-x86

Author: None (llvmbot)

Changes

Backport 143cce7

Requested by: @dtcxzyw


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

2 Files Affected:

  • (modified) llvm/lib/Target/X86/X86ISelLowering.cpp (+8-1)
  • (added) llvm/test/CodeGen/X86/pr138982.ll (+23)
diff --git a/llvm/lib/Target/X86/X86ISelLowering.cpp b/llvm/lib/Target/X86/X86ISelLowering.cpp
index 627cef9ead7ff..4413fbb77f415 100644
--- a/llvm/lib/Target/X86/X86ISelLowering.cpp
+++ b/llvm/lib/Target/X86/X86ISelLowering.cpp
@@ -54147,12 +54147,19 @@ SDValue X86TargetLowering::getNegatedExpression(SDValue Op, SelectionDAG &DAG,
     if (!Flags.hasNoSignedZeros())
       break;
 
+    // Because getCheaperNegatedExpression can delete nodes we need a handle to
+    // keep temporary nodes alive.
+    std::list<HandleSDNode> Handles;
+
     // This is always negatible for free but we might be able to remove some
     // extra operand negations as well.
     SmallVector<SDValue, 4> NewOps(Op.getNumOperands(), SDValue());
-    for (int i = 0; i != 3; ++i)
+    for (int i = 0; i != 3; ++i) {
       NewOps[i] = getCheaperNegatedExpression(
           Op.getOperand(i), DAG, LegalOperations, ForCodeSize, Depth + 1);
+      if (!!NewOps[i])
+        Handles.emplace_back(NewOps[i]);
+    }
 
     bool NegA = !!NewOps[0];
     bool NegB = !!NewOps[1];
diff --git a/llvm/test/CodeGen/X86/pr138982.ll b/llvm/test/CodeGen/X86/pr138982.ll
new file mode 100644
index 0000000000000..32346d823a9fe
--- /dev/null
+++ b/llvm/test/CodeGen/X86/pr138982.ll
@@ -0,0 +1,23 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 5
+; RUN: llc < %s -mtriple=x86_64 -mattr=+fma | FileCheck %s
+
+define <4 x float> @pr138982(<4 x float> %in_vec) {
+; CHECK-LABEL: pr138982:
+; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    vxorps {{\.?LCPI[0-9]+_[0-9]+}}(%rip), %xmm0, %xmm1
+; CHECK-NEXT:    vrcpps %xmm0, %xmm2
+; CHECK-NEXT:    vrcpps %xmm1, %xmm1
+; CHECK-NEXT:    vxorps %xmm3, %xmm3, %xmm3
+; CHECK-NEXT:    vcmpneqps %xmm0, %xmm3, %xmm0
+; CHECK-NEXT:    vbroadcastss {{.*#+}} xmm4 = [1.0E+0,1.0E+0,1.0E+0,1.0E+0]
+; CHECK-NEXT:    vblendvps %xmm0, %xmm1, %xmm4, %xmm0
+; CHECK-NEXT:    vfnmadd231ps {{.*#+}} xmm0 = -(xmm3 * xmm2) + xmm0
+; CHECK-NEXT:    retq
+entry:
+  %fneg = fneg <4 x float> %in_vec
+  %rcp = tail call <4 x float> @llvm.x86.sse.rcp.ps(<4 x float> %fneg)
+  %cmp = fcmp une <4 x float> zeroinitializer, %in_vec
+  %sel = select <4 x i1> %cmp, <4 x float> %rcp, <4 x float> splat (float 1.000000e+00)
+  %fma = call nsz <4 x float> @llvm.fma.v4f32(<4 x float> %rcp, <4 x float> zeroinitializer, <4 x float> %sel)
+  ret <4 x float> %fma
+}

@tstellar tstellar moved this from Needs Triage to Needs Review in LLVM Release Status May 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Needs Review
Development

Successfully merging this pull request may close these issues.

2 participants