From: Nirav Dave Date: Tue, 18 Jul 2017 17:39:15 +0000 (+0000) Subject: [DAG] Avoid deleting nodes before combining them. X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=e9ec240bfb4d16e4cd7dba812421fc92c851d5b4;p=llvm [DAG] Avoid deleting nodes before combining them. When replacing a node and it's operand, replacing the operand node may cause the deletion of the original node leading to an assertion failure. Case around these replacements to avoid this without relying on inspecting the DELETED_NODE opcode in various extend dagcombiner cases. Fixes PR32515. Reviewers: dbabokin, RKSimon, davide, chandlerc Subscribers: chandlerc, llvm-commits Differential Revision: https://reviews.llvm.org/D34095 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@308330 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/lib/CodeGen/SelectionDAG/DAGCombiner.cpp index d526dfd0cc4..f2ecdbe253e 100644 --- a/lib/CodeGen/SelectionDAG/DAGCombiner.cpp +++ b/lib/CodeGen/SelectionDAG/DAGCombiner.cpp @@ -7171,7 +7171,11 @@ SDValue DAGCombiner::visitSIGN_EXTEND(SDNode *N) { SDValue Trunc = DAG.getNode(ISD::TRUNCATE, SDLoc(N0), N0.getValueType(), ExtLoad); ExtendSetCCUses(SetCCs, Trunc, ExtLoad, DL, ISD::SIGN_EXTEND); - CombineTo(N0.getNode(), Trunc, ExtLoad.getValue(1)); + // If the load value is used only by N, replace it via CombineTo N. + if (N0.getValue(0).hasOneUse()) + DAG.ReplaceAllUsesOfValueWith(SDValue(LN0, 1), ExtLoad.getValue(1)); + else + CombineTo(LN0, Trunc, ExtLoad.getValue(1)); return CombineTo(N, ExtLoad); // Return N so it doesn't get rechecked! } } @@ -7229,7 +7233,11 @@ SDValue DAGCombiner::visitSIGN_EXTEND(SDNode *N) { SDLoc(N0.getOperand(0)), N0.getOperand(0).getValueType(), ExtLoad); ExtendSetCCUses(SetCCs, Trunc, ExtLoad, DL, ISD::SIGN_EXTEND); - CombineTo(N0.getOperand(0).getNode(), Trunc, ExtLoad.getValue(1)); + // If the load value is used only by N, replace it via CombineTo N. + if (SDValue(LN0, 0).hasOneUse()) + DAG.ReplaceAllUsesOfValueWith(SDValue(LN0, 1), ExtLoad.getValue(1)); + else + CombineTo(LN0, Trunc, ExtLoad.getValue(1)); return CombineTo(N, And); // Return N so it doesn't get rechecked! } } @@ -7470,7 +7478,11 @@ SDValue DAGCombiner::visitZERO_EXTEND(SDNode *N) { SDValue Trunc = DAG.getNode(ISD::TRUNCATE, SDLoc(N0), N0.getValueType(), ExtLoad); ExtendSetCCUses(SetCCs, Trunc, ExtLoad, SDLoc(N), ISD::ZERO_EXTEND); - CombineTo(N0.getNode(), Trunc, ExtLoad.getValue(1)); + // If the load value is used only by N, replace it via CombineTo N. + if (SDValue(LN0, 0).hasOneUse()) + DAG.ReplaceAllUsesOfValueWith(SDValue(LN0, 1), ExtLoad.getValue(1)); + else + CombineTo(LN0, Trunc, ExtLoad.getValue(1)); return CombineTo(N, ExtLoad); // Return N so it doesn't get rechecked! } } @@ -7522,7 +7534,11 @@ SDValue DAGCombiner::visitZERO_EXTEND(SDNode *N) { SDLoc(N0.getOperand(0)), N0.getOperand(0).getValueType(), ExtLoad); ExtendSetCCUses(SetCCs, Trunc, ExtLoad, DL, ISD::ZERO_EXTEND); - CombineTo(N0.getOperand(0).getNode(), Trunc, ExtLoad.getValue(1)); + // If the load value is used only by N, replace it via CombineTo N. + if (SDValue(LN0, 0).hasOneUse()) + DAG.ReplaceAllUsesOfValueWith(SDValue(LN0, 1), ExtLoad.getValue(1)); + else + CombineTo(LN0, Trunc, ExtLoad.getValue(1)); return CombineTo(N, And); // Return N so it doesn't get rechecked! } } @@ -7695,13 +7711,16 @@ SDValue DAGCombiner::visitANY_EXTEND(SDNode *N) { LN0->getChain(), LN0->getBasePtr(), N0.getValueType(), LN0->getMemOperand()); - CombineTo(N, ExtLoad); SDValue Trunc = DAG.getNode(ISD::TRUNCATE, SDLoc(N0), N0.getValueType(), ExtLoad); - CombineTo(N0.getNode(), Trunc, ExtLoad.getValue(1)); ExtendSetCCUses(SetCCs, Trunc, ExtLoad, SDLoc(N), ISD::ANY_EXTEND); - return SDValue(N, 0); // Return N so it doesn't get rechecked! + // If the load value is used only by N, replace it via CombineTo N. + if (N0.hasOneUse()) + DAG.ReplaceAllUsesOfValueWith(SDValue(LN0, 1), ExtLoad.getValue(1)); + else + CombineTo(LN0, Trunc, ExtLoad.getValue(1)); + return CombineTo(N, ExtLoad); // Return N so it doesn't get rechecked! } } diff --git a/test/CodeGen/X86/avx512-mask-op.ll b/test/CodeGen/X86/avx512-mask-op.ll index e1a92c60d18..6f4bf061a21 100644 --- a/test/CodeGen/X86/avx512-mask-op.ll +++ b/test/CodeGen/X86/avx512-mask-op.ll @@ -1630,8 +1630,9 @@ define void @f1(i32 %c) { ; CHECK-LABEL: f1: ; CHECK: ## BB#0: ## %entry ; CHECK-NEXT: movzbl {{.*}}(%rip), %edi -; CHECK-NEXT: movl %edi, %eax -; CHECK-NEXT: xorb $1, %al +; CHECK-NEXT: movb {{.*}}(%rip), %al +; CHECK-NEXT: notb %al +; CHECK-NEXT: andb $1, %al ; CHECK-NEXT: movb %al, {{.*}}(%rip) ; CHECK-NEXT: xorl $1, %edi ; CHECK-NEXT: jmp _f2 ## TAILCALL diff --git a/test/CodeGen/X86/pr32515.ll b/test/CodeGen/X86/pr32515.ll new file mode 100644 index 00000000000..aeb6803867a --- /dev/null +++ b/test/CodeGen/X86/pr32515.ll @@ -0,0 +1,29 @@ +; RUN: llc -O0 -mtriple=x86_64-unknown -mcpu=skx -o - %s +; RUN: llc -mtriple=x86_64-unknown -mcpu=skx -o - %s +; RUN: llc -O0 -mtriple=i686-unknown -mcpu=skx -o - %s +; RUN: llc -mtriple=i686-unknown -mcpu=skx -o - %s +; REQUIRES: asserts + +@var_26 = external global i16, align 2 + +define void @foo() #0 { + %1 = alloca i16, align 2 + %2 = load i16, i16* @var_26, align 2 + %3 = zext i16 %2 to i32 + %4 = icmp ne i32 %3, 7 + %5 = zext i1 %4 to i16 + store i16 %5, i16* %1, align 2 + %6 = load i16, i16* @var_26, align 2 + %7 = zext i16 %6 to i32 + %8 = and i32 1, %7 + %9 = shl i32 %8, 0 + %10 = load i16, i16* @var_26, align 2 + %11 = zext i16 %10 to i32 + %12 = icmp ne i32 %11, 7 + %13 = zext i1 %12 to i32 + %14 = and i32 %9, %13 + %15 = icmp ne i32 %14, 0 + %16 = zext i1 %15 to i8 + store i8 %16, i8* undef, align 1 + unreachable + }