<html><head></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><br><div><div>On Jul 22, 2009, at 8:04 AM, David Greene wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div><blockquote type="cite"><br></blockquote><br>What performance issues? I don't remember seeing anything on that. If <br>so, I'm interested in fixing them. Honestly, no one ever told me about<br>it. It's very difficult to keep up with llvm-commits without being <br>copied explicitly on important e-mails like this.<br><br><blockquote type="cite">I was slightly over-generous when handling your recent tblgen patch. <br></blockquote><blockquote type="cite">It got reverted due to correctness changes, but when Evan reported the <br></blockquote><blockquote type="cite">obvious and measured perf problems, we technically should have <br></blockquote><br>Again, I didn't see the report. Can someone send it to me?<br></div></blockquote><div><br></div><div>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.</div><div><br></div><div><span class="Apple-style-span" style="font-family: monospace; "><blockquote type="cite">On Jul 21, 2009, at 10:07 AM, Evan Cheng wrote:<br><br><blockquote type="cite">And last night's Grue-O0 test result shows this:<br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">LLC compile:<br></blockquote><blockquote type="cite">MultiSource/Applications/ClamAV/clamscan: 36.23% (1.7056 => 2.3235)<br></blockquote><blockquote type="cite">MultiSource/Applications/JM/ldecod/ldecod: 39.66% (0.9846 => 1.3751)<br></blockquote><blockquote type="cite">MultiSource/Applications/JM/lencod/lencod: 44.91% (2.0477 => 2.9673)<br></blockquote><blockquote type="cite">MultiSource/Applications/SPASS/SPASS: 26.88% (2.6105 => 3.3121)<br></blockquote><blockquote type="cite">MultiSource/Applications/d/make_dparser: 38.56% (0.3646 => 0.5052)<br></blockquote><blockquote type="cite">MultiSource/Applications/kimwitu++/kc: 27.08% (3.8236 => 4.8589)<br></blockquote><blockquote type="cite">MultiSource/Applications/lua/lua: 30.57% (0.5633 => 0.7355)<br></blockquote><blockquote type="cite">MultiSource/Applications/oggenc/oggenc: 40.32% (0.5695 => 0.7991)<br></blockquote><blockquote type="cite">MultiSource/Applications/siod/siod: 29.45% (0.3878 => 0.5020)<br></blockquote><blockquote type="cite">MultiSource/Applications/sqlite3/sqlite3: 36.53% (1.2424 => 1.6963)<br></blockquote><blockquote type="cite">MultiSource/Applications/treecc/treecc: 27.37% (0.4249 => 0.5412)<br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">David, are you planning to fix this soon? Please make sure the compile time recovers completely without -asm-verbose. Thanks.<br></blockquote><br>David, is there a reason to keep your patch in now, since you're planning to effectively redesign it?<br><br>-Chris</blockquote></span></div><div><font class="Apple-style-span" face="monospace"><br></font></div><div><font class="Apple-style-span" face="monospace"><br></font></div><blockquote type="cite"><div><br><blockquote type="cite">reverted it again. I didn't appreciate it when you reapplied the <br></blockquote><blockquote type="cite">patch (with correctness fixes) even though there were large <br></blockquote><blockquote type="cite">outstanding performance issues. It seems that it would be more <br></blockquote><blockquote type="cite">natural to fix all the problems found by review and the testers, then <br></blockquote><blockquote type="cite">re-apply it.<br></blockquote><br>How could I fix a performance issue I didn't know about? I see people <br>committing stuff with "Make changes requested by XYZ" all the time.<br></div></blockquote><div><br></div>I don't know you missed the entire thread. Your email was on recipient list.</div><div><br></div><div>Evan</div><div> <br><blockquote type="cite"><div><br>I was also thinking along the lines of incremental changes. Fix the <br>correctness issue first (which, BTW, I couldn't possibly have known <br>about given the testbase at the time) and then fix the remaining issues <br>after that. That was a deliberate choice on my part to try to comply <br>with developer policy.<br><br><blockquote type="cite">In the end, I think this patch will be end up different enough that it <br></blockquote><blockquote type="cite">should be discussed before it goes in. Specifically for this patch, <br></blockquote><blockquote type="cite">please remove all the generic "prefix handling" stuff and just make a <br></blockquote><blockquote type="cite">change localized to LiveIntervalAnalysis routine. The double-nested <br></blockquote><blockquote type="cite">for-loop is not that much code to duplicate.<br></blockquote><br>Again, it's a matter of opinion. I like more general solutions, you<br>appreciate specific ones. I don't think there's anything fundamentally<br>wrong with either. Of course I'll make the changes requested and send<br>the patch for review ahead of time.<br><br>In this particular case, I don't see how I can make a change local to <br>LiveIntervalAnalysis when the dump functions are in MachineFunction and <br>MachineBasicBlock and are reused all over the place. Maybe I'm missing <br>something. Do you have a particular solution in mind?<br><br><blockquote type="cite">of development with impact on the various developers. For changes <br></blockquote><blockquote type="cite">that add major new APIs, classes, and concepts, please discuss it <br></blockquote><blockquote type="cite">ahead of time. Trying to short-circuit the process won't make things <br></blockquote><blockquote type="cite">go faster in the end.<br></blockquote><br>I certainly wasn't trying to short-circuit anything. I honestly didn't<br>think enhancing debug dumps would be a huge deal. I understand better<br>what the goal is now and that will help.<br><br><blockquote type="cite"><blockquote type="cite"><blockquote type="cite">Please revert r76602+ and propose them to llvm-commits.<br></blockquote></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">I see someone already did. Would've been nice to get an e-mail <br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">about it.<br></blockquote></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">I assumed that you read commits, so you'd see it.<br></blockquote><br>I do read commits. I can't possibly read all of the hundreds of<br>messages that fly by every day. I try to scan and pick out the ones<br>that are most relevant to me.<br><br>Actually, what would help here is a better post-commit hook that allows<br>us to specify a subject in the commit e-mail. git does this in a nice<br>way by assuming the log begins with a short line suitable for an e-mail<br>subject line followed by a longer descriptive paragraph.<br><br>It would really help with issues like this.<br><br>Thanks for discussing this.<br><br> -Dave<br><br>_______________________________________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits<br></div></blockquote></div><br></body></html>