[DAG] check more operands for cycles when merging stores.

Until now, we've only checked whether merging stores would cause a cycle via
the value argument, but the address and indexed offset arguments are also
capable of creating cycles in some situations.

The addresses are all base+offset with notionally the same base, but the base
SDNode may still be different (e.g. via an indexed load in one case, and an
ISD::ADD elsewhere). This allows cycles to creep in if one of these sources
depends on another.

The indexed offset is usually undef (representing a non-indexed store), but on
some architectures (e.g. 32-bit ARM-mode ARM) it can be an arbitrary value,
again allowing dependency cycles to creep in.

llvm-svn: 345200
This commit is contained in:
Tim Northover 2018-10-24 21:36:34 +00:00
parent 76ccec07b4
commit 05fe8f918b
1 changed files with 8 additions and 8 deletions

View File

@ -14316,14 +14316,14 @@ bool DAGCombiner::checkMergeStoreCandidatesForDependencies(
// in candidate selection and can be
// safely ignored
// * Value (Op 1) -> Cycles may happen (e.g. through load chains)
// * Address (Op 2) -> Merged addresses may only vary by a fixed constant
// and so no cycles are possible.
// * (Op 3) -> appears to always be undef. Cannot be source of cycle.
//
// Thus we need only check predecessors of the value operands.
auto *Op = N->getOperand(1).getNode();
if (Visited.insert(Op).second)
Worklist.push_back(Op);
// * Address (Op 2) -> Merged addresses may only vary by a fixed constant,
// but aren't necessarily fromt the same base node, so
// cycles possible (e.g. via indexed store).
// * (Op 3) -> Represents the pre or post-indexing offset (or undef for
// non-indexed stores). Not constant on all targets (e.g. ARM)
// and so can participate in a cycle.
for (unsigned j = 1; j < N->getNumOperands(); ++j)
Worklist.push_back(N->getOperand(j).getNode());
}
// Search through DAG. We can stop early if we find a store node.
for (unsigned i = 0; i < NumStores; ++i)