[llvm-commits] [llvm] r168832 - in /llvm/trunk: lib/CodeGen/SelectionDAG/DAGCombiner.cpp test/CodeGen/X86/2012-11-28-merge-store-alias.ll

[Nick Lewycky]

On 28 November 2012 16:00, Nadav Rotem <nrotem at apple.com> wrote:

> Author: nadav
> Date: Wed Nov 28 18:00:08 2012
> New Revision: 168832
>
> URL: http://llvm.org/viewvc/llvm-project?rev=168832&view=rev
> Log:
> When combining consecutive stores allow loads in between the stores, if
> the loads do not alias.
>
>
> Added:
>     llvm/trunk/test/CodeGen/X86/2012-11-28-merge-store-alias.ll
> Modified:
>     llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
>
> Modified: llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp?rev=168832&r1=168831&r2=168832&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp (original)
> +++ llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp Wed Nov 28
> 18:00:08 2012
> @@ -291,6 +291,10 @@
>                   unsigned SrcValueAlign2,
>                   const MDNode *TBAAInfo2) const;
>
> +    /// isAlias - Return true if there is any possibility that the two
> addresses
> +    /// overlap.
> +    bool isAlias(LSBaseSDNode *Op0, LSBaseSDNode *Op1);
> +
>      /// FindAliasInfo - Extracts the relevant alias information from the
> memory
>      /// node.  Returns true if the operand was a load.
>      bool FindAliasInfo(SDNode *N,
> @@ -7552,7 +7556,14 @@
>    if (BasePtr.first.getOpcode() == ISD::UNDEF)
>      return false;
>
> +  // Save the LoadSDNodes that we find in the chain.
> +  // We need to make sure that these nodes do not interfere with
> +  // any of the store nodes.
> +  SmallVector<LSBaseSDNode*, 8> AliasLoadNodes;
> +
> +  // Save the StoreSDNodes that we find in the chain.
>    SmallVector<MemOpLink, 8> StoreNodes;
> +
>    // Walk up the chain and look for nodes with offsets from the same
>    // base pointer. Stop when reaching an instruction with a different kind
>    // or instruction which has a different base pointer.
> @@ -7596,8 +7607,26 @@
>      // We found a potential memory operand to merge.
>      StoreNodes.push_back(MemOpLink(Index, Ptr.second, Seq++));
>
> -    // Move up the chain to the next memory operation.
> -    Index = dyn_cast<StoreSDNode>(Index->getChain().getNode());
> +    // Find the next memory operand in the chain. If the next operand in
> the
> +    // chain is a store then move up and continue the scan with the next
> +    // memory operand. If the next operand is a load save it and use alias
> +    // information to check if it interferes with anything.
> +    SDNode *NextInChain = Index->getChain().getNode();
> +    while (1) {
> +      if (isa<StoreSDNode>(NextInChain)) {
> +        // We found a store node. Use it for the next iteration.
> +        Index = cast<StoreSDNode>(NextInChain);
> +        break;
> +      } else if (LoadSDNode *Ldn = dyn_cast<LoadSDNode>(NextInChain)) {
> +        // Save the load node for later. Continue the scan.
> +        AliasLoadNodes.push_back(Ldn);
> +        NextInChain = Ldn->getChain().getNode();
> +        continue;
> +      } else {
> +        Index = NULL;
> +        break;
> +      }
> +    }
>

This loop is more simply written:

  while (1) {
    if (LoadSDNode *Ldn = dyn_cast<LoadSDNode>(NextInChain)) {
      // Save the load node for later. Continue the scan.
      AliasLoadNodes.push_back(Ldn);
      NextInChain = Ldn->getChain().getNode();
    } else {
      // If we found a store node, use it for the next iteration.
      Index = dyn_cast<StoreSDNode>(NextInChain);
      break;
    }
  }

?

   }
>
>    // Check if there is anything to merge.
> @@ -7612,11 +7641,23 @@
>    // store memory address.
>    unsigned LastConsecutiveStore = 0;
>    int64_t StartAddress = StoreNodes[0].OffsetFromBase;
> -  for (unsigned i=1; i<StoreNodes.size(); ++i) {
> +  for (unsigned i = 1, e = StoreNodes.size(); i < e; ++i) {
>      int64_t CurrAddress = StoreNodes[i].OffsetFromBase;
>      if (CurrAddress - StartAddress != (ElementSizeBytes * i))
>        break;
>
> +    bool Alias = false;
> +    // Check if this store interferes with any of the loads that we found.
> +    for (unsigned ld = 0, lde = AliasLoadNodes.size(); ld < lde; ++ld)
> +      if (isAlias(AliasLoadNodes[ld], StoreNodes[i].MemNode)) {
> +        Alias = true;
> +        break;
> +      }
> +
> +    // We found a load that alias with this store. Stop the sequence.
> +    if (Alias)
> +      break;
> +
>      // Mark this node as useful.
>      LastConsecutiveStore = i;
>    }
> @@ -9680,6 +9721,23 @@
>    return true;
>  }
>
> +bool DAGCombiner::isAlias(LSBaseSDNode *Op0, LSBaseSDNode *Op1) {
> +  SDValue Ptr0, Ptr1;
> +  int64_t Size0, Size1;
> +  const Value *SrcValue0, *SrcValue1;
> +  int SrcValueOffset0, SrcValueOffset1;
> +  unsigned SrcValueAlign0, SrcValueAlign1;
> +  const MDNode *SrcTBAAInfo0, *SrcTBAAInfo1;
> +  FindAliasInfo(Op0, Ptr0, Size0, SrcValue0, SrcValueOffset0,
> +                SrcValueAlign0, SrcTBAAInfo0);
> +  FindAliasInfo(Op1, Ptr1, Size1, SrcValue1, SrcValueOffset1,
> +                SrcValueAlign1, SrcTBAAInfo1);
> +  return isAlias(Ptr0, Size0, SrcValue0, SrcValueOffset0,
> +               SrcValueAlign0, SrcTBAAInfo0,
> +               Ptr1, Size1, SrcValue1, SrcValueOffset1,
> +               SrcValueAlign1, SrcTBAAInfo1);
>

Wrong indent on the lines hanging after isAlias( .

Nick
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20121205/af4393aa/attachment.html>