LLVM Bugzilla is read-only and represents the historical archive of all LLVM issues filled before November 26, 2021. Use github to submit LLVM bugs

Bug 2577 - Use static profile info from __builtin_expect in optimizer to improve code
Summary: Use static profile info from __builtin_expect in optimizer to improve code
Status: RESOLVED FIXED
Alias: None
Product: new-bugs
Classification: Unclassified
Component: new bugs (show other bugs)
Version: unspecified
Hardware: All All
: P enhancement
Assignee: Unassigned LLVM Bugs
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-07-21 16:42 PDT by Daniel Dunbar
Modified: 2011-10-20 10:31 PDT (History)
11 users (show)

See Also:
Fixed By Commit(s):


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Daniel Dunbar 2008-07-21 16:42:19 PDT
We should use the information provided by the gcc __builtin_expect function.

This involves adding some mechanism for encoding the information in the LLVM IR
(most likely an intrinsic) and updating llvm-gcc and clang to emit this information.
Comment 1 Chris Lattner 2008-10-16 01:02:01 PDT
Retitling bug, we already "support" it, the idea is to use it for something.
Comment 2 Chris Lattner 2011-08-10 13:05:34 PDT
Bob, is this done?
Comment 3 Bob Wilson 2011-08-10 13:08:13 PDT
Sure, we can call it done.  We currently only use it for a few things but the infrastructure is in place, along with a few uses.
Comment 4 Chandler Carruth 2011-10-17 01:22:23 PDT
This is not yet done... It's a bit hard to prove to yourself that it is not done by running a command because we have few optimizations using the information, but I've added a testing layer that demonstrates that in fact metadata produced by the LLVM expect intrinsic does not get incorporated into the branch probability analysis, or consequentially into the block frequency analysis, or any optimizations we do have using them.

I'm mailing patches to cfe-commits which will bridge this gap in the infrastructure, and I've already fixed a critical bug in branch probabilities that would have led to turning this on producing exactly *backwards* probabilities, pessimizing code instead of optimizing it. See r142168 for that bit of fun.
Comment 5 Andrew Trick 2011-10-18 15:07:28 PDT
I explained the situation on the dev list back in August:
http://lists.cs.uiuc.edu/pipermail/llvmdev/2011-August/042863.html

It was well-known that BranchInst::swapSuccessors is broken under the current meta data scheme, and who knows what else.

I'm looking at Chandler's patch now.
Comment 6 Evan Cheng 2011-10-18 20:06:51 PDT
Cloned to rdar://10308394
Comment 7 Chandler Carruth 2011-10-19 05:40:37 PDT
Fixed with the following commits:

r142168 -- fix most egregious corruption of probability metadata
r142491 -- add testing facility to ensure we actually get expect into probabilities
r142492 -- add support for expects which feed branches to the probability analysis
r142493 -- add support for expects which feed switches to the probability analysis

We're now back to the state of 1) the probabilities haven't been completely audited, and 2) we only have a few passes that take advantage of them. But that was the desired state for closing this PR. __builtin_expect now influences code generation.


Chris, you had expressed an interest in pulling these into 3.0 -- it seems somewhat borderline to me though. I'll leave the decision to you and Bill to sort out. =]
Comment 8 Chris Lattner 2011-10-19 16:59:04 PDT
Bill, if this is actually completely broken in LLVM 3.0 and causing us to pessimize code, I'd recommend taking this.  Ultimately it's your call though.
Comment 9 Chandler Carruth 2011-10-19 17:28:41 PDT
(In reply to comment #8)
> Bill, if this is actually completely broken in LLVM 3.0 and causing us to
> pessimize code

From what I can tell, the 3.0 code is not pessimized really. While it is "completely broken" in that __builtin_expect data is completely ignored and has no impact on any parts of the optimizer, it doesn't have a negative impact.

The bugs I fixed here didn't impact the probability information (purely heuristic based) that *is* currently active in 3.0, so the bug fixes alone are not worth merging.
Comment 10 Bob Wilson 2011-10-20 10:31:38 PDT
*** Bug 11096 has been marked as a duplicate of this bug. ***