[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.
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
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).
ppc64-align-long-double.ll now may see multiple serializations
of its stores
More information about the llvm-commits