[llvm] r305468 - [DAG] Allow truncated and extend memory operations in Store Merge. NFCI.

[Nirav Davé via llvm-commits]

Yikes! It looks like in my shuffling of patches around to I merged the
functional ext load input change in with the change directly generating
legalized memory operations directly instead of waiting until legalization.
Thanks for catching it.


On Thu, Jun 15, 2017 at 7:33 PM, Ahmed Bougacha <ahmed.bougacha at gmail.com>
wrote:

> Hi Nirav,
>
> I reverted this in r305527 because it causes assertion failures (PR33475).
>
> More generally, this change (and others) look pretty functional to me.
> Is this all enabled by some future commit?  If so, the commit message
> should probably mention that this really is a functional change
> required for correctness;  it just happens to not be testable right
> now (or even better, commit everything together to avoid adding dead
> code).
>
> Thanks!
>
> -Ahmed
>
> On Thu, Jun 15, 2017 at 7:04 AM, Nirav Dave via llvm-commits
> <llvm-commits at lists.llvm.org> wrote:
> > Author: niravd
> > Date: Thu Jun 15 09:04:07 2017
> > New Revision: 305468
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=305468&view=rev
> > Log:
> > [DAG] Allow truncated and extend memory operations in Store Merge. NFCI.
> >
> > As all store merges checks are based on the memory operation
> > performed, allow use of truncated stores and extended loads as valid
> > input candidates for merging.
> >
> > 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=305468&r1=
> 305467&r2=305468&view=diff
> > ============================================================
> ==================
> > --- llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp (original)
> > +++ llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp Thu Jun 15
> 09:04:07 2017
> > @@ -12874,9 +12874,6 @@ bool DAGCombiner::MergeConsecutiveStores
> >        if (Ld->isVolatile() || Ld->isIndexed())
> >          break;
> >
> > -      // We do not accept ext loads.
> > -      if (Ld->getExtensionType() != ISD::NON_EXTLOAD)
> > -        break;
> >
> >        // The stored memory type must be the same.
> >        if (Ld->getMemoryVT() != MemVT)
> > @@ -13014,17 +13011,31 @@ bool DAGCombiner::MergeConsecutiveStores
> >
> >      // The merged loads are required to have the same incoming chain, so
> >      // using the first's chain is acceptable.
> > -    SDValue NewLoad = DAG.getLoad(JointMemOpVT, LoadDL,
> FirstLoad->getChain(),
> > -                                  FirstLoad->getBasePtr(),
> > -                                  FirstLoad->getPointerInfo(),
> FirstLoadAlign);
> >
> >      SDValue NewStoreChain = getMergeStoreChains(StoreNodes, NumElem);
> > -
> >      AddToWorklist(NewStoreChain.getNode());
> >
> > -    SDValue NewStore = DAG.getStore(
> > -        NewStoreChain, StoreDL, NewLoad, FirstInChain->getBasePtr(),
> > -        FirstInChain->getPointerInfo(), FirstStoreAlign);
> > +    SDValue NewLoad, NewStore;
> > +    if (TLI.isTypeLegal(JointMemOpVT)) {
> > +      NewLoad = DAG.getLoad(JointMemOpVT, LoadDL, FirstLoad->getChain(),
> > +                            FirstLoad->getBasePtr(),
> > +                            FirstLoad->getPointerInfo(),
> FirstLoadAlign);
> > +      NewStore = DAG.getStore(NewStoreChain, StoreDL, NewLoad,
> > +                              FirstInChain->getBasePtr(),
> > +                              FirstInChain->getPointerInfo(),
> FirstStoreAlign);
> > +    } else { // This must be the truncstore/extload case
> > +      EVT ExtendedTy =
> > +          TLI.getTypeToTransformTo(*DAG.getContext(), JointMemOpVT);
> > +      NewLoad = DAG.getExtLoad(ISD::EXTLOAD, LoadDL, ExtendedTy,
> > +                               FirstLoad->getChain(),
> FirstLoad->getBasePtr(),
> > +                               FirstLoad->getPointerInfo(),
> JointMemOpVT,
> > +                               FirstLoadAlign);
> > +      NewStore = DAG.getTruncStore(NewStoreChain, StoreDL, NewLoad,
> > +                                   FirstInChain->getBasePtr(),
> > +                                   FirstInChain->getPointerInfo(),
> JointMemOpVT,
> > +                                   FirstInChain->getAlignment(),
> > +                                   FirstInChain->getMemOperand()-
> >getFlags());
> > +    }
> >
> >      // Transfer chain users from old loads to the new load.
> >      for (unsigned i = 0; i < NumElem; ++i) {
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170615/b35209f0/attachment-0001.html>