From 369e0ef67d73345a82481c01ac9c67a590f71373 Mon Sep 17 00:00:00 2001 From: Chandler Carruth Date: Thu, 14 Aug 2014 08:18:34 +0000 Subject: [PATCH] [SDAG] Fix a bug in the DAG combiner where we would fail to return the input node after manually adding it to the worklist and using CombineTo. Once we use CombineTo the input node may have been deleted. Despite this being *completely confusing* and somewhat broken, the only way to "correctly" return from a DAG combine after potentially deleting the input node is to return *that exact node*.... But really, this code should just never have used CombineTo. It won't do what it wants (returning the node as mentioned above just causes the combine to infloop). The correct way to combine away a casted load to a load of the correct type is to RAUW the chain directly and then return the loaded value to replace the actual value node. I managed to find this with the vector shuffle fuzzer even though it clearly has nothing at all to do with vector shuffles and rather those happen to trigger a load of a constant pool that hits this combine *just right*. I've included the test as it is small and a nice stress test that the infrastructure isn't asserting. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@215622 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/CodeGen/SelectionDAG/DAGCombiner.cpp | 6 +----- test/CodeGen/X86/vector-shuffle-128-v16.ll | 24 ++++++++++++++++++++++ 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/lib/CodeGen/SelectionDAG/DAGCombiner.cpp index 603ccb090ed..4f898bc9d7a 100644 --- a/lib/CodeGen/SelectionDAG/DAGCombiner.cpp +++ b/lib/CodeGen/SelectionDAG/DAGCombiner.cpp @@ -6303,11 +6303,7 @@ SDValue DAGCombiner::visitBITCAST(SDNode *N) { LN0->isVolatile(), LN0->isNonTemporal(), LN0->isInvariant(), OrigAlign, LN0->getAAInfo()); - AddToWorklist(N); - CombineTo(N0.getNode(), - DAG.getNode(ISD::BITCAST, SDLoc(N0), - N0.getValueType(), Load), - Load.getValue(1)); + DAG.ReplaceAllUsesOfValueWith(N0.getValue(1), Load.getValue(1)); return Load; } } diff --git a/test/CodeGen/X86/vector-shuffle-128-v16.ll b/test/CodeGen/X86/vector-shuffle-128-v16.ll index ad512402a33..913389382fa 100644 --- a/test/CodeGen/X86/vector-shuffle-128-v16.ll +++ b/test/CodeGen/X86/vector-shuffle-128-v16.ll @@ -289,3 +289,27 @@ entry: %s.16.0 = shufflevector <16 x i8> %s.7.1, <16 x i8> %s.7.2, <16 x i32> ret <16 x i8> %s.16.0 } + +define <16 x i8> @stress_test1(<16 x i8> %s.0.5, <16 x i8> %s.0.8, <16 x i8> %s.0.9) noinline nounwind { +; We don't have anything useful to check here. This generates tons of +; instructions. Instead, just make sure we survived codegen. +; +; ALL-LABEL: @stress_test1 +; ALL: retq +entry: + %s.1.8 = shufflevector <16 x i8> %s.0.8, <16 x i8> undef, <16 x i32> + %s.2.4 = shufflevector <16 x i8> undef, <16 x i8> %s.0.5, <16 x i32> + %s.2.5 = shufflevector <16 x i8> %s.0.5, <16 x i8> undef, <16 x i32> + %s.2.9 = shufflevector <16 x i8> %s.0.9, <16 x i8> undef, <16 x i32> + %s.3.4 = shufflevector <16 x i8> %s.2.4, <16 x i8> %s.0.5, <16 x i32> + %s.3.9 = shufflevector <16 x i8> %s.2.9, <16 x i8> undef, <16 x i32> + %s.4.7 = shufflevector <16 x i8> %s.1.8, <16 x i8> %s.2.9, <16 x i32> + %s.4.8 = shufflevector <16 x i8> %s.2.9, <16 x i8> %s.3.9, <16 x i32> + %s.5.7 = shufflevector <16 x i8> %s.4.7, <16 x i8> %s.4.8, <16 x i32> + %s.8.4 = shufflevector <16 x i8> %s.3.4, <16 x i8> %s.5.7, <16 x i32> + %s.9.4 = shufflevector <16 x i8> %s.8.4, <16 x i8> undef, <16 x i32> + %s.10.4 = shufflevector <16 x i8> %s.9.4, <16 x i8> undef, <16 x i32> + %s.12.4 = shufflevector <16 x i8> %s.10.4, <16 x i8> undef, <16 x i32> + + ret <16 x i8> %s.12.4 +} -- 2.34.1