[PATCH] D18062: Fix Load Control Dependence in MemCpy Generation

Nirav Dave via llvm-commits llvm-commits at lists.llvm.org
Mon Mar 21 11:29:51 PDT 2016


niravd added a subscriber: niravd.
niravd added a comment.

Oops. I didn't send this to the mailing list so the summary is missing.
Adding below:

In Memcpy lowering we had missed a dependence from the load of the
operation to successor operations. This causes us to potentially construct
an in initial DAG with a memory dependence not fully represented in the
chain sub-DAG but rather require looking at the entire DAG breaking alias
analysis by allowing incorrect repositioning of memory operations.

To work around this, r200033 changed DAGCombiner::GatherAllAliases to be
conservative if any possible issues to happen. Unfortunately this check
forbade many non-problematic situations as well. For example, it's common
for incoming argument lowering to add a non-aliasing load hanging off of
EntryNode. Then, if GatherAllAliases visited EntryNode, it would find that
other (unvisited) use of the EntryNode chain, and just give up entirely.
Furthermore, the check was incomplete: it would not actually detect all
such potentially problematic DAG constructions, because GatherAllAliases
did not guarantee to visit all chain nodes going up to the root EntryNode.
This is in general fine -- giving up early will just miss a potential
optimization, not generate incorrect results. But, for this non-chain
dependency detection code, it's possible that you could have a load
attached to a higher-up chain node than any which were visited. If that
load aliases your store, but the only dependency is through the value
operand of a non-aliasing store, it would've been missed by this code, and
potentially reordered.

With the dependence added, this check can be removed and Alas Analysis can
be much more aggressive. This fixes code quality regression in the
Consecutive Store Merge cleanup (http://reviews.llvm.org/D14834).

Test Change:

ppc64-align-long-double.ll now may see multiple serializations
of its stores


http://reviews.llvm.org/D18062





More information about the llvm-commits mailing list