llvm dev - Apr 2012 - [LLVMdev] Switching the new block placement pass on by default for 3.1

Hello folks,

I think I've fixed everything left to be fixed in block placement, and I
would really like for it to be on by default in 3.1:
1) I've been testing this as thoroughly as I can across a large amount of
real-world code, so it shouldn't be too flaky.
2) The code has been checked in and reviewed previously.
3) It completes a significant missing feature in LLVM with builtin_expect.
4) It "fixes" some of the regressions introduced with the inlining
work by
more effectively ensuring the loops are laid out correctly. (yacr2
specifically exhibited this problem)
5) I see more positive movement in performance than negative, but that may
be an artifact of the testing I've been doing.

I'm planning to check in a change to enable it by default later tonight
(PST) and hopefully allow nightly testers to run through it and report any
significant problems it introduces. It should be relatively easy to revert
if folks are opposed to it being in 3.1, or if the testers turn up any
problems*.

Thoughts? Others agree with turning it on? Strong desire for it to be
delayed until after the branch? I do understand the desire to avoid last
minute features... I just wasn't able to isolate the single big performance
regression I saw until today, despite working on it for two weeks... ::
sigh ::

-Chandler

[*]: There is one problem I'm ready for -- the code is very assert heavy.
FNT runs on other architectures may turn up other invalid asserts. I'll be
watching build bots for this case.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20120416/009a2720/attachment.html>

On Apr 15, 2012, at 6:42 PM, Chandler Carruth wrote:
> Hello folks,
> 
> I think I've fixed everything left to be fixed in block placement, and
I would really like for it to be on by default in 3.1:
> 1) I've been testing this as thoroughly as I can across a large amount
of real-world code, so it shouldn't be too flaky.
> 2) The code has been checked in and reviewed previously.
> 3) It completes a significant missing feature in LLVM with builtin_expect.
> 4) It "fixes" some of the regressions introduced with the
inlining work by more effectively ensuring the loops are laid out correctly.
(yacr2 specifically exhibited this problem)
> 5) I see more positive movement in performance than negative, but that may
be an artifact of the testing I've been doing.
> 
> I'm planning to check in a change to enable it by default later tonight
(PST) and hopefully allow nightly testers to run through it and report any
significant problems it introduces. It should be relatively easy to revert if
folks are opposed to it being in 3.1, or if the testers turn up any problems*.
> 
> Thoughts? Others agree with turning it on? Strong desire for it to be
delayed until after the branch? I do understand the desire to avoid last minute
features... I just wasn't able to isolate the single big performance
regression I saw until today, despite working on it for two weeks... :: sigh ::
> 
Sooner rather than later is obviously better. I would say turn it on for a
nightly test to make sure it works for everyone.

-bw

On Mon, Apr 16, 2012 at 9:00 AM, Bill Wendling <wendling at apple.com>
wrote:
> On Apr 15, 2012, at 6:42 PM, Chandler Carruth wrote:
>
> > Hello folks,
> >
> > I think I've fixed everything left to be fixed in block placement,
and I
> would really like for it to be on by default in 3.1:
> > 1) I've been testing this as thoroughly as I can across a large
amount
> of real-world code, so it shouldn't be too flaky.
> > 2) The code has been checked in and reviewed previously.
> > 3) It completes a significant missing feature in LLVM with
> builtin_expect.
> > 4) It "fixes" some of the regressions introduced with the
inlining work
> by more effectively ensuring the loops are laid out correctly. (yacr2
> specifically exhibited this problem)
> > 5) I see more positive movement in performance than negative, but that
> may be an artifact of the testing I've been doing.
> >
> > I'm planning to check in a change to enable it by default later
tonight
> (PST) and hopefully allow nightly testers to run through it and report any
> significant problems it introduces. It should be relatively easy to revert
> if folks are opposed to it being in 3.1, or if the testers turn up any
> problems*.
> >
> > Thoughts? Others agree with turning it on? Strong desire for it to be
> delayed until after the branch? I do understand the desire to avoid last
> minute features... I just wasn't able to isolate the single big
performance
> regression I saw until today, despite working on it for two weeks... ::
> sigh ::
> >
> Sooner rather than later is obviously better. I would say turn it on for a
> nightly test to make sure it works for everyone.

I've enabled it in r154816. I tried to get it done sooner, but updating the
regression tests was a nightmare. That said, they uncovered two super easy
missed optimizations implemented along the way.

I'll be around for quite some time and will be watching build bots. I've
already started a local nightly test run on my end as well.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20120416/d1734f6c/attachment.html>