[X86][TargetLowering] Avoid deleting temporary nodes in getNegatedExpression

[dtcxzyw]

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 #138944.

[llvmbot]

@llvm/pr-subscribers-backend-x86

Author: Yingwei Zheng (dtcxzyw)

Changes

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 #138944.

---

Full diff: https://github.com/llvm/llvm-project/pull/139029.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 483aceb239b0c..767f3eaceab2f 100644
--- a/llvm/lib/Target/X86/X86ISelLowering.cpp
+++ b/llvm/lib/Target/X86/X86ISelLowering.cpp
@@ -54678,12 +54678,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
+}

[dtcxzyw]

std::optional<HandleSDNode> Handles[3] may be better.

[wjristow]

The fix LGTM, and I can confirm it fixes my original, un-reduced test-case.

I don't have an opinion on:

std::optional<HandleSDNode> Handles[3] may be better.

[topperc]

LGTM

[dtcxzyw]

/cherry-pick 143cce7

[llvmbot]

/pull-request #139356

[llvm-ci]

LLVM Buildbot has detected a new failure on builder premerge-monolithic-windows running on premerge-windows-1 while building llvm at step 5 "clean-build-dir".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/35/builds/9958

Here is the relevant piece of the build log for the reference

Step 5 (clean-build-dir) failure: Delete failed. (failure)
Step 8 (test-build-unified-tree-check-all) failure: test (failure)
******************** TEST 'lit :: timeout-hang.py' FAILED ********************
Exit Code: 1

Command Output (stdout):
--
# RUN: at line 13
not env -u FILECHECK_OPTS "C:\Python39\python.exe" C:\ws\buildbot\premerge-monolithic-windows\llvm-project\llvm\utils\lit\lit.py -j1 --order=lexical Inputs/timeout-hang/run-nonexistent.txt  --timeout=1 --param external=0 | "C:\Python39\python.exe" C:\ws\buildbot\premerge-monolithic-windows\build\utils\lit\tests\timeout-hang.py 1
# executed command: not env -u FILECHECK_OPTS 'C:\Python39\python.exe' 'C:\ws\buildbot\premerge-monolithic-windows\llvm-project\llvm\utils\lit\lit.py' -j1 --order=lexical Inputs/timeout-hang/run-nonexistent.txt --timeout=1 --param external=0
# .---command stderr------------
# | lit.py: C:\ws\buildbot\premerge-monolithic-windows\llvm-project\llvm\utils\lit\lit\main.py:72: note: The test suite configuration requested an individual test timeout of 0 seconds but a timeout of 1 seconds was requested on the command line. Forcing timeout to be 1 seconds.
# `-----------------------------
# executed command: 'C:\Python39\python.exe' 'C:\ws\buildbot\premerge-monolithic-windows\build\utils\lit\tests\timeout-hang.py' 1
# .---command stdout------------
# | Testing took as long or longer than timeout
# `-----------------------------
# error: command failed with exit status: 1

--

********************