[llvm-commits] [llvm] r76615 - in /llvm/trunk: include/llvm/CodeGen/LiveIntervalAnalysis.h lib/CodeGen/LiveIntervalAnalysis.cpp

[Evan Cheng]

On Jul 22, 2009, at 8:04 AM, David Greene wrote:

>>
>
> What performance issues?  I don't remember seeing anything on that.   
> If
> so, I'm interested in fixing them.  Honestly, no one ever told me  
> about
> it.  It's very difficult to keep up with llvm-commits without being
> copied explicitly on important e-mails like this.
>
>> I was slightly over-generous when handling your recent tblgen patch.
>> It got reverted due to correctness changes, but when Evan reported  
>> the
>> obvious and measured perf problems, we technically should have
>
> Again, I didn't see the report.  Can someone send it to me?

You were explicitly cc'd on this email. Your patch 76486 slowed down  
compilation across the board. I'd prefer you rip it out until you have  
fixed the severe performance problem.

> On Jul 21, 2009, at 10:07 AM, Evan Cheng wrote:
>
>> And last night's Grue-O0 test result shows this:
>>
>> LLC compile:
>> MultiSource/Applications/ClamAV/clamscan: 36.23% (1.7056 => 2.3235)
>> MultiSource/Applications/JM/ldecod/ldecod: 39.66% (0.9846 => 1.3751)
>> MultiSource/Applications/JM/lencod/lencod: 44.91% (2.0477 => 2.9673)
>> MultiSource/Applications/SPASS/SPASS: 26.88% (2.6105 => 3.3121)
>> MultiSource/Applications/d/make_dparser: 38.56% (0.3646 => 0.5052)
>> MultiSource/Applications/kimwitu++/kc: 27.08% (3.8236 => 4.8589)
>> MultiSource/Applications/lua/lua: 30.57% (0.5633 => 0.7355)
>> MultiSource/Applications/oggenc/oggenc: 40.32% (0.5695 => 0.7991)
>> MultiSource/Applications/siod/siod: 29.45% (0.3878 => 0.5020)
>> MultiSource/Applications/sqlite3/sqlite3: 36.53% (1.2424 => 1.6963)
>> MultiSource/Applications/treecc/treecc: 27.37% (0.4249 => 0.5412)
>>
>> David, are you planning to fix this soon? Please make sure the  
>> compile time recovers completely without -asm-verbose. Thanks.
>
> David, is there a reason to keep your patch in now, since you're  
> planning to effectively redesign it?
>
> -Chris



>
>> reverted it again.  I didn't appreciate it when you reapplied the
>> patch (with correctness fixes) even though there were large
>> outstanding performance issues.  It seems that it would be more
>> natural to fix all the problems found by review and the testers, then
>> re-apply it.
>
> How could I fix a performance issue I didn't know about?  I see people
> committing stuff with "Make changes requested by XYZ" all the time.

I don't know you missed the entire thread. Your email was on recipient  
list.

Evan

>
> I was also thinking along the lines of incremental changes.  Fix the
> correctness issue first (which, BTW, I couldn't possibly have known
> about given the testbase at the time) and then fix the remaining  
> issues
> after that.  That was a deliberate choice on my part to try to comply
> with developer policy.
>
>> In the end, I think this patch will be end up different enough that  
>> it
>> should be discussed before it goes in.  Specifically for this patch,
>> please remove all the generic "prefix handling" stuff and just make a
>> change localized to LiveIntervalAnalysis routine.  The double-nested
>> for-loop is not that much code to duplicate.
>
> Again, it's a matter of opinion.  I like more general solutions, you
> appreciate specific ones.  I don't think there's anything  
> fundamentally
> wrong with either.  Of course I'll make the changes requested and send
> the patch for review ahead of time.
>
> In this particular case, I don't see how I can make a change local to
> LiveIntervalAnalysis when the dump functions are in MachineFunction  
> and
> MachineBasicBlock and are reused all over the place.  Maybe I'm  
> missing
> something.  Do you have a particular solution in mind?
>
>> of development with impact on the various developers.  For changes
>> that add major new APIs, classes, and concepts, please discuss it
>> ahead of time.  Trying to short-circuit the process won't make things
>> go faster in the end.
>
> I certainly wasn't trying to short-circuit anything.  I honestly  
> didn't
> think enhancing debug dumps would be a huge deal.  I understand better
> what the goal is now and that will help.
>
>>>> Please revert r76602+ and propose them to llvm-commits.
>>> I see someone already did.  Would've been nice to get an e-mail
>>> about it.
>>
>> I assumed that you read commits, so you'd see it.
>
> I do read commits.  I can't possibly read all of the hundreds of
> messages that fly by every day.  I try to scan and pick out the ones
> that are most relevant to me.
>
> Actually, what would help here is a better post-commit hook that  
> allows
> us to specify a subject in the commit e-mail.  git does this in a nice
> way by assuming the log begins with a short line suitable for an e- 
> mail
> subject line followed by a longer descriptive paragraph.
>
> It would really help with issues like this.
>
> Thanks for discussing this.
>
>                            -Dave
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20090722/ef75a20c/attachment.html>