[llvm-commits] PATCH: Refactoring and simplification of inline cost computation w.r.t. alloca arguments

[Chandler Carruth]

On Wed, Mar 7, 2012 at 2:08 PM, Eric Christopher <echristo at apple.com> wrote:

>
> On Mar 7, 2012, at 5:38 AM, Chandler Carruth <chandlerc at gmail.com> wrote:
>
> > Hello,
> >
> > 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.
> >
> >
> > 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.
> >
>
> 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?
>

No, i've bootstrapped clang and it made no difference in either. I don't
really expect it to except in relatively rare circumstances.

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.

I've attached a version of the patch with a testcase now. =]

> 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?
>
> ... only 2 patches were attached :)
>

Sorry for confusing wording, this was referring to the second patch.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120307/f588f05c/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-Undo-a-previous-restriction-on-the-inline-cost-calcu.patch
Type: application/octet-stream
Size: 12367 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120307/f588f05c/attachment.obj>