From de3ced2d10fa245ae0040753c8b1c857cf720b97 Mon Sep 17 00:00:00 2001 From: Marcin Koscielnicki Date: Mon, 25 Apr 2016 15:43:44 +0000 Subject: [PATCH] [PR27390] [CodeGen] Reject indexed loads in CombinerDAG. visitAND, when folding and (load) forgets to check which output of an indexed load is involved, happily folding the updated address output on the following testcase: target datalayout = "e-m:e-i64:64-n32:64" target triple = "powerpc64le-unknown-linux-gnu" %typ = type { i32, i32 } define signext i32 @_Z8access_pP1Tc(%typ* %p, i8 zeroext %type) { %b = getelementptr inbounds %typ, %typ* %p, i64 0, i32 1 %1 = load i32, i32* %b, align 4 %2 = ptrtoint i32* %b to i64 %3 = and i64 %2, -35184372088833 %4 = inttoptr i64 %3 to i32* %_msld = load i32, i32* %4, align 4 %zzz = add i32 %1, %_msld ret i32 %zzz } Fix this by checking ResNo. I've found a few more places that currently neglect to check for indexed load, and tightened them up as well, but I don't have test cases for them. In fact, they might not be triggerable at all, at least with current targets. Still, better safe than sorry. Differential Revision: http://reviews.llvm.org/D19202 llvm-svn: 267420 --- lib/CodeGen/SelectionDAG/DAGCombiner.cpp | 11 ++++++--- lib/CodeGen/SelectionDAG/SelectionDAG.cpp | 2 ++ test/CodeGen/PowerPC/2016-04-17-combine.ll | 26 ++++++++++++++++++++++ 3 files changed, 36 insertions(+), 3 deletions(-) create mode 100644 test/CodeGen/PowerPC/2016-04-17-combine.ll diff --git a/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/lib/CodeGen/SelectionDAG/DAGCombiner.cpp index f740e59af96..71ab653230f 100644 --- a/lib/CodeGen/SelectionDAG/DAGCombiner.cpp +++ b/lib/CodeGen/SelectionDAG/DAGCombiner.cpp @@ -963,7 +963,8 @@ void DAGCombiner::ReplaceLoadWithPromotedLoad(SDNode *Load, SDNode *ExtLoad) { SDValue DAGCombiner::PromoteOperand(SDValue Op, EVT PVT, bool &Replace) { Replace = false; SDLoc dl(Op); - if (LoadSDNode *LD = dyn_cast(Op)) { + if (ISD::isUNINDEXEDLoad(Op.getNode())) { + LoadSDNode *LD = cast(Op); EVT MemVT = LD->getMemoryVT(); ISD::LoadExtType ExtType = ISD::isNON_EXTLoad(LD) ? (TLI.isLoadExtLegal(ISD::ZEXTLOAD, PVT, MemVT) ? ISD::ZEXTLOAD @@ -1167,6 +1168,9 @@ bool DAGCombiner::PromoteLoad(SDValue Op) { if (!LegalOperations) return false; + if (!ISD::isUNINDEXEDLoad(Op.getNode())) + return false; + EVT VT = Op.getValueType(); if (VT.isVector() || !VT.isInteger()) return false; @@ -3140,8 +3144,9 @@ SDValue DAGCombiner::visitAND(SDNode *N) { // more cases. if ((N0.getOpcode() == ISD::EXTRACT_VECTOR_ELT && N0.getValueSizeInBits() == N0.getOperand(0).getScalarValueSizeInBits() && - N0.getOperand(0).getOpcode() == ISD::LOAD) || - N0.getOpcode() == ISD::LOAD) { + N0.getOperand(0).getOpcode() == ISD::LOAD && + N0.getOperand(0).getResNo() == 0) || + (N0.getOpcode() == ISD::LOAD && N0.getResNo() == 0)) { LoadSDNode *Load = cast( (N0.getOpcode() == ISD::LOAD) ? N0 : N0.getOperand(0) ); diff --git a/lib/CodeGen/SelectionDAG/SelectionDAG.cpp b/lib/CodeGen/SelectionDAG/SelectionDAG.cpp index deab0e472f6..6446c462be7 100644 --- a/lib/CodeGen/SelectionDAG/SelectionDAG.cpp +++ b/lib/CodeGen/SelectionDAG/SelectionDAG.cpp @@ -6961,6 +6961,8 @@ bool SelectionDAG::areNonVolatileConsecutiveLoads(LoadSDNode *LD, int Dist) const { if (LD->isVolatile() || Base->isVolatile()) return false; + if (LD->isIndexed() || Base->isIndexed()) + return false; if (LD->getChain() != Base->getChain()) return false; EVT VT = LD->getValueType(0); diff --git a/test/CodeGen/PowerPC/2016-04-17-combine.ll b/test/CodeGen/PowerPC/2016-04-17-combine.ll new file mode 100644 index 00000000000..7ad943cf35f --- /dev/null +++ b/test/CodeGen/PowerPC/2016-04-17-combine.ll @@ -0,0 +1,26 @@ +; RUN: llc <%s | FileCheck %s +target datalayout = "e-m:e-i64:64-n32:64" +target triple = "powerpc64le-unknown-linux-gnu" + +; PR27390 crasher + +%typ = type { i32, i32 } + +; On release builds, it doesn't crash, spewing nonsense instead. +; To make sure it works, check that and is still alive. +; CHECK: and +; Also, in release, it emits a COPY from a 32-bit register to +; a 64-bit register, which happens to be emitted as cror [!] +; by the confused CodeGen. Just to be sure, check there isn't one. +; CHECK-NOT: cror +; Function Attrs: uwtable +define signext i32 @_Z8access_pP1Tc(%typ* %p, i8 zeroext %type) { + %b = getelementptr inbounds %typ, %typ* %p, i64 0, i32 1 + %1 = load i32, i32* %b, align 4 + %2 = ptrtoint i32* %b to i64 + %3 = and i64 %2, -35184372088833 + %4 = inttoptr i64 %3 to i32* + %_msld = load i32, i32* %4, align 4 + %zzz = add i32 %1, %_msld + ret i32 %zzz +}