<div dir="ltr">Hi Hal,<div><br></div><div>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? </div><div><br></div><div>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?</div><div><br></div><div>Thanks,</div><div>-Jiangning</div><div><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">2014-09-16 2:51 GMT+08:00 Hal Finkel <span dir="ltr"><<a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</a>></span>:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">----- Original Message -----<br>
> From: "Jiangning Liu" <<a href="mailto:liujiangning1@gmail.com">liujiangning1@gmail.com</a>><br>
> To: <a href="mailto:liujiangning1@gmail.com">liujiangning1@gmail.com</a>, <a href="mailto:hfinkel@anl.gov">hfinkel@anl.gov</a><br>
> Cc: <a href="mailto:mcrosier@codeaurora.org">mcrosier@codeaurora.org</a>, joerg@NetBSD.org, <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
> Sent: Sunday, September 14, 2014 10:18:49 PM<br>
> Subject: Re: [PATCH] Add threshold for lowering lattice value 'overdefined' in LVI<br>
><br>
> Hi Hal, Jeorg and Chad,<br>
><br>
> Thanks for your feedback!<br>
><br>
> -Jiangning<br>
><br>
> ================<br>
> Comment at: lib/Analysis/LazyValueInfo.cpp:41<br>
> @@ -39,1 +40,3 @@<br>
><br>
> +// This threshold is purticularly tuned with some special cases<br>
> exposing<br>
> +// regression of compile time and memory consumption.<br>
> ----------------<br>
> joerg wrote:<br>
> > Typo. Maybe with the changes suggsted by Hal use just:<br>
> ><br>
> > // Experimentally derived threshold for additional lowering lattice<br>
> > values per block<br>
> OK.<br>
><br>
> ================<br>
> Comment at: lib/Analysis/LazyValueInfo.cpp:360<br>
> @@ -351,1 +359,3 @@<br>
> +    /// lowered.<br>
> +    unsigned LoweringOverdefinedTimes;<br>
><br>
> ----------------<br>
> hfinkel wrote:<br>
> > Could you make this a DenseMap<BasicBlock *, unsigned>, and keep a<br>
> > count per block?<br>
> OK. But for this test case, the algorithm just encounters too many<br>
> basic blocks rather than too many times for a single basic block.<br>
> Anyway, to avoid confusion, following your comments, I added another<br>
> threshold to check the number of basic blocks, so now we have two<br>
> thresholds.<br>
<br>
</div></div>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.<br>
<br>
Thanks again,<br>
Hal<br>
<div class="HOEnZb"><div class="h5"><br>
><br>
><br>
><br>
> ================<br>
> Comment at: lib/Analysis/LazyValueInfo.cpp:556<br>
> @@ -542,1 +555,3 @@<br>
> +  // complexity. Therefore, we introduce OverdefinedThreshold to<br>
> control<br>
> +  // the maximum times of further lowering lattice value<br>
> 'overdefined'.<br>
><br>
> ----------------<br>
> joerg wrote:<br>
> > Maybe just:<br>
> ><br>
> > // Limit the number of additional lowering lattice value to avoid<br>
> > unjustified memory grows.<br>
> OK.<br>
><br>
> ================<br>
> Comment at: lib/Analysis/LazyValueInfo.cpp:574<br>
> @@ -556,2 +573,3 @@<br>
>    BBLV.markOverdefined();<br>
> +  LoweringOverdefinedTimes++;<br>
><br>
> ----------------<br>
> mcrosier wrote:<br>
> > Please use a preincrement.<br>
> ><br>
> > <a href="http://llvm.org/releases/2.7/docs/CodingStandards.html#micro_preincrement" target="_blank">http://llvm.org/releases/2.7/docs/CodingStandards.html#micro_preincrement</a><br>
> OK.<br>
><br>
> <a href="http://reviews.llvm.org/D5322" target="_blank">http://reviews.llvm.org/D5322</a><br>
><br>
><br>
><br>
<br>
</div></div><span class="HOEnZb"><font color="#888888">--<br>
Hal Finkel<br>
Assistant Computational Scientist<br>
Leadership Computing Facility<br>
Argonne National Laboratory<br>
</font></span></blockquote></div><br></div>