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

[Hal Finkel via llvm-commits]

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

 -Hal

> 
> -- Sanjoy
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> 

-- 
Hal Finkel
Lead, Compiler Technology and Programming Languages
Leadership Computing Facility
Argonne National Laboratory