[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).

llvm-svn: 237576
This commit is contained in:
Hal Finkel 2015-05-18 15:46:02 +00:00
parent 482b9d0ef8
commit a60e633fdd
1 changed files with 12 additions and 11 deletions

View File

@ -9155,24 +9155,25 @@ bool DAGCombiner::CombineToPreIndexedLoadStore(SDNode *N) {
// a copy of the original base pointer. // a copy of the original base pointer.
SmallVector<SDNode *, 16> OtherUses; SmallVector<SDNode *, 16> OtherUses;
if (isa<ConstantSDNode>(Offset)) if (isa<ConstantSDNode>(Offset))
for (SDNode *Use : BasePtr.getNode()->uses()) { for (SDNode::use_iterator UI = BasePtr.getNode()->use_begin(),
if (Use == Ptr.getNode()) 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; continue;
if (Use->isPredecessorOf(N)) if (Use.getUser()->isPredecessorOf(N))
continue; continue;
if (Use->getOpcode() != ISD::ADD && Use->getOpcode() != ISD::SUB) { if (Use.getUser()->getOpcode() != ISD::ADD &&
Use.getUser()->getOpcode() != ISD::SUB) {
OtherUses.clear(); OtherUses.clear();
break; break;
} }
SDValue Op0 = Use->getOperand(0), Op1 = Use->getOperand(1); SDValue Op1 = Use.getUser()->getOperand((UI.getOperandNo() + 1) & 1);
if (Op1.getNode() == BasePtr.getNode())
std::swap(Op0, Op1);
assert(Op0.getNode() == BasePtr.getNode() &&
"Use of ADD/SUB but not an operand");
if (!isa<ConstantSDNode>(Op1)) { if (!isa<ConstantSDNode>(Op1)) {
OtherUses.clear(); OtherUses.clear();
break; break;
@ -9184,7 +9185,7 @@ bool DAGCombiner::CombineToPreIndexedLoadStore(SDNode *N) {
break; break;
} }
OtherUses.push_back(Use); OtherUses.push_back(Use.getUser());
} }
if (Swapped) if (Swapped)