[LLVMdev] DAGCombiner::MergeConsecutiveStores

[Mikael Holmén]

Hi,

I'm quite puzzled by a little bit of code in the DAGCombiner where it 
merges loads in MergeConsecutiveStores.

Two 16bit loads have been merged to one 32bit load, and two 16bit stores 
have been combined to one 32bit store.

And then the code goes like this:


   // Replace one of the loads with the new load.
   LoadSDNode *Ld = cast<LoadSDNode>(LoadNodes[0].MemNode);
   DAG.ReplaceAllUsesOfValueWith(SDValue(Ld, 1),
                                 SDValue(NewLoad.getNode(), 1));

   // Remove the rest of the load chains.
   for (unsigned i = 1; i < NumElem ; ++i) {
     // Replace all chain users of the old load nodes with the chain of 
the new
     // load node.
     LoadSDNode *Ld = cast<LoadSDNode>(LoadNodes[i].MemNode);
     DAG.ReplaceAllUsesOfValueWith(SDValue(Ld, 1), Ld->getChain());
   }

And here I can't understand why we should replace "the rest" of the load 
chains with the loads' getChain(). Why should one load be treated in one 
way, and the rest in some other way?

I.e. if there is a chain dependendy to a load, we replace that with the 
load's chain. Why not replace dependencies to the old loads with 
dependencies to the new load, just like we do for the first load in the 
code above.

This code was rewritten to how it looks today in a commit 2012-10-03:

"    Fix a cycle in the DAG. In this code we replace multiple loads
      with a single load and
      multiple stores with a single load. We create the wide loads and
      stores (and their chains)
      before we remove the scalar loads and stores and fix the DAG chain.
      We attempted to merge loads with a different chain. When that
      happened, the assumption that it is safe to RAUW
      broke and a cycle was introduced.

     git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@165148 
91177308-0d34-0410-b5e6-96231b3b80d8
"

The testcase in that commit works even if I treat all loads the same and 
replace them all with SDValue(NewLoad.getNode(), 1)

Anyone knows this code and why it looks the way it does?

Regards,
Mikael