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

[Michael Spencer]

On Thu, Oct 23, 2014 at 1:03 PM, Hal Finkel <hfinkel at anl.gov> wrote:
> ----- Original Message -----
>> From: "Michael Spencer" <bigcheesegs at gmail.com>
>> To: "Hal Finkel" <hfinkel at anl.gov>
>> Cc: "Chandler Carruth" <chandlerc at google.com>, "amara emerson" <amara.emerson at arm.com>, "llvm-commits"
>> <llvm-commits at cs.uiuc.edu>, "ahmed bougacha" <ahmed.bougacha at gmail.com>, "david majnemer"
>> <david.majnemer at gmail.com>, reviews+D5898+public+a9255b02ad589d6d at reviews.llvm.org
>> Sent: Thursday, October 23, 2014 3:00:46 PM
>> Subject: Re: [PATCH] [InstCombine] Fold endian-independent load sequence into a single load.
>>
>> 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.
>
> Why would this transformation not cause similar regressions?
>
>  -Hal

Because it is specifically targeting the
load-bytes-from-memory-as-integral-and-maybe-bswap idiom. The load
combiner targets any adjacent loads anywhere, even if they are going
to be used separately directly after that.

- Michael Spencer

>
>>
>> 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