Skip to content

Commit bff59ac

Browse files
topperctstellar
authored andcommitted
[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 74e6030)
1 parent a123bea commit bff59ac

File tree

2 files changed

+84
-0
lines changed

2 files changed

+84
-0
lines changed

llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5935,6 +5935,11 @@ SDValue TargetLowering::getNegatedExpression(SDValue Op, SelectionDAG &DAG,
59355935

59365936
SDLoc DL(Op);
59375937

5938+
// Because getNegatedExpression can delete nodes we need a handle to keep
5939+
// temporary nodes alive in case the recursion manages to create an identical
5940+
// node.
5941+
std::list<HandleSDNode> Handles;
5942+
59385943
switch (Opcode) {
59395944
case ISD::ConstantFP: {
59405945
// Don't invert constant FP values after legalization unless the target says
@@ -6003,11 +6008,18 @@ SDValue TargetLowering::getNegatedExpression(SDValue Op, SelectionDAG &DAG,
60036008
NegatibleCost CostX = NegatibleCost::Expensive;
60046009
SDValue NegX =
60056010
getNegatedExpression(X, DAG, LegalOps, OptForSize, CostX, Depth);
6011+
// Prevent this node from being deleted by the next call.
6012+
if (NegX)
6013+
Handles.emplace_back(NegX);
6014+
60066015
// fold (fneg (fadd X, Y)) -> (fsub (fneg Y), X)
60076016
NegatibleCost CostY = NegatibleCost::Expensive;
60086017
SDValue NegY =
60096018
getNegatedExpression(Y, DAG, LegalOps, OptForSize, CostY, Depth);
60106019

6020+
// We're done with the handles.
6021+
Handles.clear();
6022+
60116023
// Negate the X if its cost is less or equal than Y.
60126024
if (NegX && (CostX <= CostY)) {
60136025
Cost = CostX;
@@ -6052,11 +6064,18 @@ SDValue TargetLowering::getNegatedExpression(SDValue Op, SelectionDAG &DAG,
60526064
NegatibleCost CostX = NegatibleCost::Expensive;
60536065
SDValue NegX =
60546066
getNegatedExpression(X, DAG, LegalOps, OptForSize, CostX, Depth);
6067+
// Prevent this node from being deleted by the next call.
6068+
if (NegX)
6069+
Handles.emplace_back(NegX);
6070+
60556071
// fold (fneg (fmul X, Y)) -> (fmul X, (fneg Y))
60566072
NegatibleCost CostY = NegatibleCost::Expensive;
60576073
SDValue NegY =
60586074
getNegatedExpression(Y, DAG, LegalOps, OptForSize, CostY, Depth);
60596075

6076+
// We're done with the handles.
6077+
Handles.clear();
6078+
60606079
// Negate the X if its cost is less or equal than Y.
60616080
if (NegX && (CostX <= CostY)) {
60626081
Cost = CostX;
@@ -6094,15 +6113,25 @@ SDValue TargetLowering::getNegatedExpression(SDValue Op, SelectionDAG &DAG,
60946113
if (!NegZ)
60956114
break;
60966115

6116+
// Prevent this node from being deleted by the next two calls.
6117+
Handles.emplace_back(NegZ);
6118+
60976119
// fold (fneg (fma X, Y, Z)) -> (fma (fneg X), Y, (fneg Z))
60986120
NegatibleCost CostX = NegatibleCost::Expensive;
60996121
SDValue NegX =
61006122
getNegatedExpression(X, DAG, LegalOps, OptForSize, CostX, Depth);
6123+
// Prevent this node from being deleted by the next call.
6124+
if (NegX)
6125+
Handles.emplace_back(NegX);
6126+
61016127
// fold (fneg (fma X, Y, Z)) -> (fma X, (fneg Y), (fneg Z))
61026128
NegatibleCost CostY = NegatibleCost::Expensive;
61036129
SDValue NegY =
61046130
getNegatedExpression(Y, DAG, LegalOps, OptForSize, CostY, Depth);
61056131

6132+
// We're done with the handles.
6133+
Handles.clear();
6134+
61066135
// Negate the X if its cost is less or equal than Y.
61076136
if (NegX && (CostX <= CostY)) {
61086137
Cost = std::min(CostX, CostZ);

llvm/test/CodeGen/X86/pr49393.ll

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
2+
; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu | FileCheck %s
3+
4+
define void @f() {
5+
; CHECK-LABEL: f:
6+
; CHECK: # %bb.0: # %entry
7+
; CHECK-NEXT: xorl %eax, %eax
8+
; CHECK-NEXT: .p2align 4, 0x90
9+
; CHECK-NEXT: .LBB0_1: # %for.cond
10+
; CHECK-NEXT: # =>This Inner Loop Header: Depth=1
11+
; CHECK-NEXT: imull %eax, %eax
12+
; CHECK-NEXT: movsd {{.*#+}} xmm0 = mem[0],zero
13+
; CHECK-NEXT: movapd %xmm0, %xmm1
14+
; CHECK-NEXT: mulsd %xmm0, %xmm1
15+
; CHECK-NEXT: subsd %xmm0, %xmm1
16+
; CHECK-NEXT: cwtl
17+
; CHECK-NEXT: xorps %xmm2, %xmm2
18+
; CHECK-NEXT: cvtsi2sd %eax, %xmm2
19+
; CHECK-NEXT: mulsd %xmm0, %xmm2
20+
; CHECK-NEXT: mulsd %xmm0, %xmm2
21+
; CHECK-NEXT: movapd %xmm2, %xmm3
22+
; CHECK-NEXT: mulsd %xmm1, %xmm3
23+
; CHECK-NEXT: mulsd %xmm0, %xmm2
24+
; CHECK-NEXT: subsd %xmm3, %xmm1
25+
; CHECK-NEXT: addsd %xmm2, %xmm1
26+
; CHECK-NEXT: cvttsd2si %xmm1, %eax
27+
; CHECK-NEXT: jmp .LBB0_1
28+
entry:
29+
br label %for.cond
30+
31+
for.cond: ; preds = %for.cond, %entry
32+
%b.0 = phi i16 [ 0, %entry ], [ %conv77, %for.cond ]
33+
%mul18 = mul i16 %b.0, %b.0
34+
%arrayidx.real = load double, double* undef, align 1
35+
%arrayidx.imag = load double, double* undef, align 1
36+
%mul_ac = fmul fast double %arrayidx.real, %arrayidx.real
37+
%0 = fadd fast double 0.000000e+00, %arrayidx.real
38+
%sub.r = fsub fast double %mul_ac, %0
39+
%sub.i = fsub fast double 0.000000e+00, %arrayidx.imag
40+
%conv28 = sitofp i16 %mul18 to double
41+
%mul_bc32 = fmul fast double %arrayidx.imag, %conv28
42+
%mul_bd46 = fmul fast double %mul_bc32, %arrayidx.imag
43+
%mul_r49 = fsub fast double 0.000000e+00, %mul_bd46
44+
%mul_ac59 = fmul fast double %mul_r49, %sub.r
45+
%mul_bc48 = fmul fast double %mul_bc32, %arrayidx.real
46+
%mul_i50 = fadd fast double 0.000000e+00, %mul_bc48
47+
%1 = fmul fast double %mul_i50, %sub.i
48+
%.neg = fneg fast double %0
49+
%.neg19 = fmul fast double %1, -1.000000e+00
50+
%.neg20 = fadd fast double %.neg, %mul_ac
51+
%2 = fadd fast double %.neg20, %mul_ac59
52+
%sub.r75 = fadd fast double %2, %.neg19
53+
%conv77 = fptosi double %sub.r75 to i16
54+
br label %for.cond
55+
}

0 commit comments

Comments
 (0)