[llvm] r283612 - [InstCombine] Don't unpack arrays that are too large (part 2).

Davide Italiano via llvm-commits llvm-commits at lists.llvm.org
Tue Feb 7 10:09:22 PST 2017


On Mon, Oct 31, 2016 at 8:53 AM, Hal Finkel <hfinkel at anl.gov> wrote:
> ----- Original Message -----
>> From: "Sanjoy Das via llvm-commits" <llvm-commits at lists.llvm.org>
>> To: "Davide Italiano" <davide at freebsd.org>
>> Cc: "llvm-commits" <llvm-commits at lists.llvm.org>
>> Sent: Sunday, October 30, 2016 11:26:29 PM
>> Subject: Re: [llvm] r283612 - [InstCombine] Don't unpack arrays that are too large (part 2).
>>
>> Sorry, had lost track of this thread.
>>
>> On 10/11/16, 8:49 AM, Davide Italiano wrote:
>> > On Mon, Oct 10, 2016 at 7:04 PM, Sanjoy Das
>> > <sanjoy at playingwithpointers.com>  wrote:
>> >> How about making the 1024 a cl::opt tunable parameter (also in
>> >> https://reviews.llvm.org/rL283599)?
>> >>
>> >> -- Sanjoy
>> >
>> > I have no strong opinion about this. A quick (probably
>> > non-exaustive)
>> > search in lib/Transform/InstCombine/* shows that the only cl::opt
>> > is
>> > the one to enable/disable expensive combines, so adding
>> > threshold-based tunables doesn't seem quite common there.
>> > I'm fine with adding the new tunable but maybe we should make more
>> > generic? Something like "max size of the array we should consider
>> > when
>> > performing transforms"? At that point we could also use it inside
>> > `Instruction *InstCombiner::foldCmpLoadFromIndexedGlobal` which
>> > hardcodes a similar threshold. What do you think?
>>
>> That sounds good.
>
> I agree. In general, I think we should make an effort to encode all thresholds as cl::opt defaults. This makes in-the-field experimentation and tuning much easier.
>

Terribly late, but better than never. This is r294323.

--
Davide


More information about the llvm-commits mailing list