[PATCH] Add threshold for lowering lattice value 'overdefined' in LVI

[Jiangning Liu]

Hi Hal,

I don't understand why putting the count inside LVILatticeVal can be
helpful to the dilemma you mentioned? Can you explain more about this? And
why it can be more local that way?

I think at present we have an unique ValueCacheEntryTy for each value on a
specific block. If we put a count inside LVILatticeVal, what is the
semantic of this count?

Thanks,
-Jiangning


2014-09-16 2:51 GMT+08:00 Hal Finkel <hfinkel at anl.gov>:

> ----- Original Message -----
> > From: "Jiangning Liu" <liujiangning1 at gmail.com>
> > To: liujiangning1 at gmail.com, hfinkel at anl.gov
> > Cc: mcrosier at codeaurora.org, joerg at NetBSD.org, llvm-commits at cs.uiuc.edu
> > Sent: Sunday, September 14, 2014 10:18:49 PM
> > Subject: Re: [PATCH] Add threshold for lowering lattice value
> 'overdefined' in LVI
> >
> > Hi Hal, Jeorg and Chad,
> >
> > Thanks for your feedback!
> >
> > -Jiangning
> >
> > ================
> > Comment at: lib/Analysis/LazyValueInfo.cpp:41
> > @@ -39,1 +40,3 @@
> >
> > +// This threshold is purticularly tuned with some special cases
> > exposing
> > +// regression of compile time and memory consumption.
> > ----------------
> > joerg wrote:
> > > Typo. Maybe with the changes suggsted by Hal use just:
> > >
> > > // Experimentally derived threshold for additional lowering lattice
> > > values per block
> > OK.
> >
> > ================
> > Comment at: lib/Analysis/LazyValueInfo.cpp:360
> > @@ -351,1 +359,3 @@
> > +    /// lowered.
> > +    unsigned LoweringOverdefinedTimes;
> >
> > ----------------
> > hfinkel wrote:
> > > Could you make this a DenseMap<BasicBlock *, unsigned>, and keep a
> > > count per block?
> > OK. But for this test case, the algorithm just encounters too many
> > basic blocks rather than too many times for a single basic block.
> > Anyway, to avoid confusion, following your comments, I added another
> > threshold to check the number of basic blocks, so now we have two
> > thresholds.
>
> Alright. Here's what I'm trying to avoid: Given a large function,
> optimizing code near the beginning of the function better than code at the
> end of the function. Maybe we should put the count inside of LVILatticeVal
> itself? That seems like it should be more local.
>
> Thanks again,
> Hal
>
> >
> >
> >
> > ================
> > Comment at: lib/Analysis/LazyValueInfo.cpp:556
> > @@ -542,1 +555,3 @@
> > +  // complexity. Therefore, we introduce OverdefinedThreshold to
> > control
> > +  // the maximum times of further lowering lattice value
> > 'overdefined'.
> >
> > ----------------
> > joerg wrote:
> > > Maybe just:
> > >
> > > // Limit the number of additional lowering lattice value to avoid
> > > unjustified memory grows.
> > OK.
> >
> > ================
> > Comment at: lib/Analysis/LazyValueInfo.cpp:574
> > @@ -556,2 +573,3 @@
> >    BBLV.markOverdefined();
> > +  LoweringOverdefinedTimes++;
> >
> > ----------------
> > mcrosier wrote:
> > > Please use a preincrement.
> > >
> > >
> http://llvm.org/releases/2.7/docs/CodingStandards.html#micro_preincrement
> > OK.
> >
> > http://reviews.llvm.org/D5322
> >
> >
> >
>
> --
> Hal Finkel
> Assistant Computational Scientist
> Leadership Computing Facility
> Argonne National Laboratory
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140916/37f9f2ae/attachment.html>