[PATCH] [InstCombine] Fold endian-independent load sequence into a single load.

[Hal Finkel]

----- Original Message -----
> From: "Ahmed Bougacha" <ahmed.bougacha at gmail.com>
> To: "Michael Spencer" <bigcheesegs at gmail.com>
> Cc: "Hal Finkel" <hfinkel at anl.gov>, "Chandler Carruth" <chandlerc at google.com>, "amara emerson"
> <amara.emerson at arm.com>, "llvm-commits" <llvm-commits at cs.uiuc.edu>, "david majnemer" <david.majnemer at gmail.com>,
> reviews+D5898+public+a9255b02ad589d6d at reviews.llvm.org
> Sent: Thursday, October 23, 2014 4:01:43 PM
> Subject: Re: [PATCH] [InstCombine] Fold endian-independent load sequence into a single load.
> 
> On Thu, Oct 23, 2014 at 1:00 PM, Michael Spencer
> <bigcheesegs at gmail.com> wrote:
> > On Thu, Oct 23, 2014 at 11:48 AM, Hal Finkel <hfinkel at anl.gov>
> > wrote:
> >> Chandler, Michael, before we go farther, could you please comment
> >> on whether this is an optimal place for this optimization? We do
> >> have a LoadCombine pass, maybe it should really go there?
> >>
> >> Ahmed, I think we also need to check for other uses of the
> >> loads... if they also have other uses, then we likely don't want
> >> to double-load the same data.
> >>
> >>  -Hal
> >
> > The load combiner was made to deal with this case, however it's not
> > enabled right now because load slicing still needs to be
> > implemented
> > to deal with some of the performance regressions.
> 
> I didn't know about the load combiner; if you have any pointers, I
> can
> try to look into it rather than continuing with this (less general)
> patch then?

This is not an answer to your question, but I believe that the current LoadCombiner implementation does not handle the bswap case, which you've implemented here, and would be nice to have.

 -Hal

> 
> - Ahmed
> 
> > Right now it's in the proper place. The issue is that when we do
> > enable load combining, this will be redundant.
> >
> > - Michael Spencer
> >
> >>
> >> ----- Original Message -----
> >>> From: "Ahmed Bougacha" <ahmed.bougacha at gmail.com>
> >>> To: "ahmed bougacha" <ahmed.bougacha at gmail.com>, "david majnemer"
> >>> <david.majnemer at gmail.com>, hfinkel at anl.gov
> >>> Cc: "amara emerson" <amara.emerson at arm.com>,
> >>> llvm-commits at cs.uiuc.edu
> >>> Sent: Wednesday, October 22, 2014 1:58:47 PM
> >>> Subject: Re: [PATCH] [InstCombine] Fold endian-independent load
> >>> sequence into a single load.
> >>>
> >>> Keep the alignment from the first byte load instruction.
> >>> Also, refactor the ByteLoads handling to make it
> >>> endian-independent
> >>> (from a memory standpoint).
> >>>
> >>> The previous patch relied on the later InstCombines to set the
> >>> alignment, but we can also do it here. I didn't do that from the
> >>> beginning because I couldn't think of a case where that's
> >>> possible
> >>> but the later InstCombine isn't. I still can't, but that's no
> >>> reason
> >>> not to do it here!
> >>>
> >>> http://reviews.llvm.org/D5898
> >>>
> >>> Files:
> >>>   lib/Transforms/InstCombine/InstCombine.h
> >>>   lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
> >>>   test/Transforms/InstCombine/endian-independent-load-BE.ll
> >>>   test/Transforms/InstCombine/endian-independent-load-LE-aliasing.ll
> >>>   test/Transforms/InstCombine/endian-independent-load-LE.ll
> >>>
> >>
> >> --
> >> Hal Finkel
> >> Assistant Computational Scientist
> >> Leadership Computing Facility
> >> Argonne National Laboratory
> 

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory