<div class="gmail_quote">On Wed, Mar 7, 2012 at 2:08 PM, Eric Christopher <span dir="ltr"><<a href="mailto:echristo@apple.com">echristo@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="im"><br>
On Mar 7, 2012, at 5:38 AM, Chandler Carruth <<a href="mailto:chandlerc@gmail.com">chandlerc@gmail.com</a>> wrote:<br>
<br>
> Hello,<br>
><br>
> The first patch is a pure refactoring designed to ease the upcoming work I'm doing on responding to paired alloca arguments to functions in the inline cost assessment. It also (IMO) make the code more cohesive as the counting is entirely specific to the function info the cost analyzer is tracking.<br>

><br>
><br>
> The second patch is a requested change / refactoring from Nick. When talking with him, he indicated that his comments about the handling of icmp instructions in inline cost counting were wrong, and that we should in fact be accounting for all of the icmp reductions regardless of whether SROA would fire for an alloca pointer in that argument. This patch factors all of this logic out into separate functions with clear names, and changes the logic to eagerly compute reductions for icmp instructions, and to ensure we look at all icmp instructions in the function even if SROA is not viable.<br>

><br>
<br>
</div>I like these two in general. I worry a bit about the lack of Science!(tm) to see how we're doing on code size or performance etc after. Have any numbers from the test suite?<br></blockquote><div><br></div><div>
No, i've bootstrapped clang and it made no difference in either. I don't really expect it to except in relatively rare circumstances.</div><div><br></div><div>Is this needed before I check in? I was hoping to just look for regressions in the nightly testing rather than trying to run each incremental step through the test suite.... Hopefully doing the latter will be much easier in the future.</div>
<div><br></div><div>I've attached a version of the patch with a testcase now. =]</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="im">> This last patch I'm a bit hesitant about. I don't have any test cases for it, and during my changes I made several typos and bugs that didn't actually cause any tests to fail. I feel like the testing here is... rather lighter than it should be. I don't want to commit this one without test cases, can anyone provide some?<br>

<br>
</div>... only 2 patches were attached :)<br></blockquote><div><br></div><div>Sorry for confusing wording, this was referring to the second patch.</div></div>