[PATCH] D12775: Use new TokenFactor chain when merging stores

Matt Arsenault via llvm-commits llvm-commits at lists.llvm.org
Thu Sep 10 14:16:36 PDT 2015


arsenm created this revision.
arsenm added a reviewer: hfinkel.
arsenm added a subscriber: llvm-commits.

If the stores are storing values from loads which partially
alias the stores, we could end up placing the merged loads
and stores on the same chain which has the potential to break.
Each store may have a different chain dependency on only some
of the original loads. Create a new TokenFactor to capture all
of the required dependencies of the stores rather than assuming
all stores can use the same chain.
    
The testcase is a situation where this happens, although
it does not have an observable change from this. The DAG nodes
just happened to not be reordered before despite this missing
chain dependency.
    
This is based on an off-list report for an out of tree target
which regressed due to r246307 and I haven't managed to find a case
where the nodes do end up reordered with an in tree target.


http://reviews.llvm.org/D12775

Files:
  lib/CodeGen/SelectionDAG/DAGCombiner.cpp
  test/CodeGen/X86/2012-11-28-merge-store-alias.ll
  test/CodeGen/X86/merge-store-partially-alias-loads.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D12775.34491.patch
Type: text/x-patch
Size: 4855 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150910/a9ba4121/attachment.bin>


More information about the llvm-commits mailing list