Author: Craig Topper Date: 2021-03-08T20:15:54-08:00 New Revision: bff59aca162ef16d7634dc9df39f1f3af31ecb93
URL: https://github.com/llvm/llvm-project/commit/bff59aca162ef16d7634dc9df39f1f3af31ecb93 DIFF: https://github.com/llvm/llvm-project/commit/bff59aca162ef16d7634dc9df39f1f3af31ecb93.diff LOG: [TargetLowering] Use HandleSDNodes to prevent nodes from being deleted by recursive calls in getNegatedExpression. For binary or ternary ops we call getNegatedExpression multiple times and then compare costs. While we're doing this we need to hold a node from the first call across the second call, but its not yet attached to the DAG. Its possible the second call creates an identical node and then decides it didn't need it so will try to delete it if it has no uses. This can cause a reference to the node we're holding further up the call stack to become invalidated. To prevent this, we can use a HandleSDNode to artifically give the node a use without connecting it to the DAG. I've used a std::list of HandleSDNodes so we can create handles only when we have a node to hold. HandleSDNode does not have default constructor and cannot be copied or moved. Fixes PR49393. Reviewed By: spatel Differential Revision: https://reviews.llvm.org/D97914 (cherry picked from commit 74e6030bcbcc8e628f9a99a424342a0c656456f9) Added: llvm/test/CodeGen/X86/pr49393.ll Modified: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp Removed: ################################################################################ diff --git a/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp b/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp index 7145fc91d5f3..b0ad86899d25 100644 --- a/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp +++ b/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp @@ -5935,6 +5935,11 @@ SDValue TargetLowering::getNegatedExpression(SDValue Op, SelectionDAG &DAG, SDLoc DL(Op); + // Because getNegatedExpression can delete nodes we need a handle to keep + // temporary nodes alive in case the recursion manages to create an identical + // node. + std::list<HandleSDNode> Handles; + switch (Opcode) { case ISD::ConstantFP: { // Don't invert constant FP values after legalization unless the target says @@ -6003,11 +6008,18 @@ SDValue TargetLowering::getNegatedExpression(SDValue Op, SelectionDAG &DAG, NegatibleCost CostX = NegatibleCost::Expensive; SDValue NegX = getNegatedExpression(X, DAG, LegalOps, OptForSize, CostX, Depth); + // Prevent this node from being deleted by the next call. + if (NegX) + Handles.emplace_back(NegX); + // fold (fneg (fadd X, Y)) -> (fsub (fneg Y), X) NegatibleCost CostY = NegatibleCost::Expensive; SDValue NegY = getNegatedExpression(Y, DAG, LegalOps, OptForSize, CostY, Depth); + // We're done with the handles. + Handles.clear(); + // Negate the X if its cost is less or equal than Y. if (NegX && (CostX <= CostY)) { Cost = CostX; @@ -6052,11 +6064,18 @@ SDValue TargetLowering::getNegatedExpression(SDValue Op, SelectionDAG &DAG, NegatibleCost CostX = NegatibleCost::Expensive; SDValue NegX = getNegatedExpression(X, DAG, LegalOps, OptForSize, CostX, Depth); + // Prevent this node from being deleted by the next call. + if (NegX) + Handles.emplace_back(NegX); + // fold (fneg (fmul X, Y)) -> (fmul X, (fneg Y)) NegatibleCost CostY = NegatibleCost::Expensive; SDValue NegY = getNegatedExpression(Y, DAG, LegalOps, OptForSize, CostY, Depth); + // We're done with the handles. + Handles.clear(); + // Negate the X if its cost is less or equal than Y. if (NegX && (CostX <= CostY)) { Cost = CostX; @@ -6094,15 +6113,25 @@ SDValue TargetLowering::getNegatedExpression(SDValue Op, SelectionDAG &DAG, if (!NegZ) break; + // Prevent this node from being deleted by the next two calls. + Handles.emplace_back(NegZ); + // fold (fneg (fma X, Y, Z)) -> (fma (fneg X), Y, (fneg Z)) NegatibleCost CostX = NegatibleCost::Expensive; SDValue NegX = getNegatedExpression(X, DAG, LegalOps, OptForSize, CostX, Depth); + // Prevent this node from being deleted by the next call. + if (NegX) + Handles.emplace_back(NegX); + // fold (fneg (fma X, Y, Z)) -> (fma X, (fneg Y), (fneg Z)) NegatibleCost CostY = NegatibleCost::Expensive; SDValue NegY = getNegatedExpression(Y, DAG, LegalOps, OptForSize, CostY, Depth); + // We're done with the handles. + Handles.clear(); + // Negate the X if its cost is less or equal than Y. if (NegX && (CostX <= CostY)) { Cost = std::min(CostX, CostZ); diff --git a/llvm/test/CodeGen/X86/pr49393.ll b/llvm/test/CodeGen/X86/pr49393.ll new file mode 100644 index 000000000000..9952b90fc7b7 --- /dev/null +++ b/llvm/test/CodeGen/X86/pr49393.ll @@ -0,0 +1,55 @@ +; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py +; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu | FileCheck %s + +define void @f() { +; CHECK-LABEL: f: +; CHECK: # %bb.0: # %entry +; CHECK-NEXT: xorl %eax, %eax +; CHECK-NEXT: .p2align 4, 0x90 +; CHECK-NEXT: .LBB0_1: # %for.cond +; CHECK-NEXT: # =>This Inner Loop Header: Depth=1 +; CHECK-NEXT: imull %eax, %eax +; CHECK-NEXT: movsd {{.*#+}} xmm0 = mem[0],zero +; CHECK-NEXT: movapd %xmm0, %xmm1 +; CHECK-NEXT: mulsd %xmm0, %xmm1 +; CHECK-NEXT: subsd %xmm0, %xmm1 +; CHECK-NEXT: cwtl +; CHECK-NEXT: xorps %xmm2, %xmm2 +; CHECK-NEXT: cvtsi2sd %eax, %xmm2 +; CHECK-NEXT: mulsd %xmm0, %xmm2 +; CHECK-NEXT: mulsd %xmm0, %xmm2 +; CHECK-NEXT: movapd %xmm2, %xmm3 +; CHECK-NEXT: mulsd %xmm1, %xmm3 +; CHECK-NEXT: mulsd %xmm0, %xmm2 +; CHECK-NEXT: subsd %xmm3, %xmm1 +; CHECK-NEXT: addsd %xmm2, %xmm1 +; CHECK-NEXT: cvttsd2si %xmm1, %eax +; CHECK-NEXT: jmp .LBB0_1 +entry: + br label %for.cond + +for.cond: ; preds = %for.cond, %entry + %b.0 = phi i16 [ 0, %entry ], [ %conv77, %for.cond ] + %mul18 = mul i16 %b.0, %b.0 + %arrayidx.real = load double, double* undef, align 1 + %arrayidx.imag = load double, double* undef, align 1 + %mul_ac = fmul fast double %arrayidx.real, %arrayidx.real + %0 = fadd fast double 0.000000e+00, %arrayidx.real + %sub.r = fsub fast double %mul_ac, %0 + %sub.i = fsub fast double 0.000000e+00, %arrayidx.imag + %conv28 = sitofp i16 %mul18 to double + %mul_bc32 = fmul fast double %arrayidx.imag, %conv28 + %mul_bd46 = fmul fast double %mul_bc32, %arrayidx.imag + %mul_r49 = fsub fast double 0.000000e+00, %mul_bd46 + %mul_ac59 = fmul fast double %mul_r49, %sub.r + %mul_bc48 = fmul fast double %mul_bc32, %arrayidx.real + %mul_i50 = fadd fast double 0.000000e+00, %mul_bc48 + %1 = fmul fast double %mul_i50, %sub.i + %.neg = fneg fast double %0 + %.neg19 = fmul fast double %1, -1.000000e+00 + %.neg20 = fadd fast double %.neg, %mul_ac + %2 = fadd fast double %.neg20, %mul_ac59 + %sub.r75 = fadd fast double %2, %.neg19 + %conv77 = fptosi double %sub.r75 to i16 + br label %for.cond +} _______________________________________________ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits