From b6f6eee8fd3f0ef8df8d25b85126394588adb1d5 Mon Sep 17 00:00:00 2001 From: Hal Finkel Date: Mon, 18 May 2015 15:46:02 +0000 Subject: [PATCH] [DAGCombine] Be more pedantic about use iteration in CombineToPreIndexedLoadStore In CombineToPreIndexedLoadStore, when the offset is a constant, we have code that looks for other uses of the pointer which are constant offset computations so that they can be rewritten in terms of the updated pointer so that we don't need to keep a copy of the base pointer to compute these constant offsets. Unfortunately, when it iterated over the uses, it did so by SDNodes, and so we could confuse ourselves if the base pointer was produced by a node that had multiple results (because we would not immediately exclude uses of the other node results). This was reported as PR22755. Unfortunately, we don't have a test case (and I've also been unable to produce one thus far), but at least the mistake is clear. The right way to fix this problem is to make use of the information contained in the use iterators to filter out any uses of other results of the node producing the base pointer. This should be mostly NFC, but should also fix PR22755 (for which, unfortunately, we have no in-tree test case). git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@237576 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/CodeGen/SelectionDAG/DAGCombiner.cpp | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/lib/CodeGen/SelectionDAG/DAGCombiner.cpp index 3728168e693..eebefb32eb0 100644 --- a/lib/CodeGen/SelectionDAG/DAGCombiner.cpp +++ b/lib/CodeGen/SelectionDAG/DAGCombiner.cpp @@ -9155,24 +9155,25 @@ bool DAGCombiner::CombineToPreIndexedLoadStore(SDNode *N) { // a copy of the original base pointer. SmallVector OtherUses; if (isa(Offset)) - for (SDNode *Use : BasePtr.getNode()->uses()) { - if (Use == Ptr.getNode()) + for (SDNode::use_iterator UI = BasePtr.getNode()->use_begin(), + UE = BasePtr.getNode()->use_end(); + UI != UE; ++UI) { + SDUse &Use = UI.getUse(); + // Skip the use that is Ptr and uses of other results from BasePtr's + // node (important for nodes that return multiple results). + if (Use.getUser() == Ptr.getNode() || Use != BasePtr) continue; - if (Use->isPredecessorOf(N)) + if (Use.getUser()->isPredecessorOf(N)) continue; - if (Use->getOpcode() != ISD::ADD && Use->getOpcode() != ISD::SUB) { + if (Use.getUser()->getOpcode() != ISD::ADD && + Use.getUser()->getOpcode() != ISD::SUB) { OtherUses.clear(); break; } - SDValue Op0 = Use->getOperand(0), Op1 = Use->getOperand(1); - if (Op1.getNode() == BasePtr.getNode()) - std::swap(Op0, Op1); - assert(Op0.getNode() == BasePtr.getNode() && - "Use of ADD/SUB but not an operand"); - + SDValue Op1 = Use.getUser()->getOperand((UI.getOperandNo() + 1) & 1); if (!isa(Op1)) { OtherUses.clear(); break; @@ -9184,7 +9185,7 @@ bool DAGCombiner::CombineToPreIndexedLoadStore(SDNode *N) { break; } - OtherUses.push_back(Use); + OtherUses.push_back(Use.getUser()); } if (Swapped)