Skip to content

Commit 74e6030

Browse files
committed
[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
1 parent 931a3aa commit 74e6030

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
@@ -5987,6 +5987,11 @@ SDValue TargetLowering::getNegatedExpression(SDValue Op, SelectionDAG &DAG,
59875987

59885988
SDLoc DL(Op);
59895989

5990+
// Because getNegatedExpression can delete nodes we need a handle to keep
5991+
// temporary nodes alive in case the recursion manages to create an identical
5992+
// node.
5993+
std::list<HandleSDNode> Handles;
5994+
59905995
switch (Opcode) {
59915996
case ISD::ConstantFP: {
59925997
// Don't invert constant FP values after legalization unless the target says
@@ -6055,11 +6060,18 @@ SDValue TargetLowering::getNegatedExpression(SDValue Op, SelectionDAG &DAG,
60556060
NegatibleCost CostX = NegatibleCost::Expensive;
60566061
SDValue NegX =
60576062
getNegatedExpression(X, DAG, LegalOps, OptForSize, CostX, Depth);
6063+
// Prevent this node from being deleted by the next call.
6064+
if (NegX)
6065+
Handles.emplace_back(NegX);
6066+
60586067
// fold (fneg (fadd X, Y)) -> (fsub (fneg Y), X)
60596068
NegatibleCost CostY = NegatibleCost::Expensive;
60606069
SDValue NegY =
60616070
getNegatedExpression(Y, DAG, LegalOps, OptForSize, CostY, Depth);
60626071

6072+
// We're done with the handles.
6073+
Handles.clear();
6074+
60636075
// Negate the X if its cost is less or equal than Y.
60646076
if (NegX && (CostX <= CostY)) {
60656077
Cost = CostX;
@@ -6104,11 +6116,18 @@ SDValue TargetLowering::getNegatedExpression(SDValue Op, SelectionDAG &DAG,
61046116
NegatibleCost CostX = NegatibleCost::Expensive;
61056117
SDValue NegX =
61066118
getNegatedExpression(X, DAG, LegalOps, OptForSize, CostX, Depth);
6119+
// Prevent this node from being deleted by the next call.
6120+
if (NegX)
6121+
Handles.emplace_back(NegX);
6122+
61076123
// fold (fneg (fmul X, Y)) -> (fmul X, (fneg Y))
61086124
NegatibleCost CostY = NegatibleCost::Expensive;
61096125
SDValue NegY =
61106126
getNegatedExpression(Y, DAG, LegalOps, OptForSize, CostY, Depth);
61116127

6128+
// We're done with the handles.
6129+
Handles.clear();
6130+
61126131
// Negate the X if its cost is less or equal than Y.
61136132
if (NegX && (CostX <= CostY)) {
61146133
Cost = CostX;
@@ -6146,15 +6165,25 @@ SDValue TargetLowering::getNegatedExpression(SDValue Op, SelectionDAG &DAG,
61466165
if (!NegZ)
61476166
break;
61486167

6168+
// Prevent this node from being deleted by the next two calls.
6169+
Handles.emplace_back(NegZ);
6170+
61496171
// fold (fneg (fma X, Y, Z)) -> (fma (fneg X), Y, (fneg Z))
61506172
NegatibleCost CostX = NegatibleCost::Expensive;
61516173
SDValue NegX =
61526174
getNegatedExpression(X, DAG, LegalOps, OptForSize, CostX, Depth);
6175+
// Prevent this node from being deleted by the next call.
6176+
if (NegX)
6177+
Handles.emplace_back(NegX);
6178+
61536179
// fold (fneg (fma X, Y, Z)) -> (fma X, (fneg Y), (fneg Z))
61546180
NegatibleCost CostY = NegatibleCost::Expensive;
61556181
SDValue NegY =
61566182
getNegatedExpression(Y, DAG, LegalOps, OptForSize, CostY, Depth);
61576183

6184+
// We're done with the handles.
6185+
Handles.clear();
6186+
61586187
// Negate the X if its cost is less or equal than Y.
61596188
if (NegX && (CostX <= CostY)) {
61606189
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)