On 11/13/18 8:42 PM, Jason Merrill wrote:
> On Tue, Nov 13, 2018 at 9:20 AM Martin Liška <mli...@suse.cz> wrote:
>>
>> On 11/13/18 5:43 AM, Jason Merrill wrote:
>>> [[likely]] and [[unlikely]] are equivalent to the GNU hot/cold attributes,
>>> except that they can be applied to arbitrary statements as well as labels;
>>> this is most likely to be useful for marking if/else branches as likely or
>>> unlikely. Conveniently, PREDICT_EXPR fits the bill nicely as a
>>> representation.
>>>
>>> I also had to fix marking case labels as hot/cold, which didn't work before.
>>> Which then required me to force __attribute ((fallthrough)) to apply to the
>>> statement rather than the label.
>>>
>>> Tested x86_64-pc-linux-gnu. Does this seem like a sane implementation
>>> approach to people with more experience with PREDICT_EXPR?
>>
>> Hi.
>>
>> In general it makes sense to implement it the same way. Question is how much
>> should the hold/cold attribute should be close to __builtin_expect.
>>
>> Let me present few examples and differences that I see:
>>
>> 1) ./xgcc -B. -O2 -fdump-tree-profile_estimate=/dev/stdout /tmp/test1.C
>>
>> ;; Function foo (_Z3foov, funcdef_no=0, decl_uid=2301, cgraph_uid=1,
>> symbol_order=3)
>>
>> Predictions for bb 2
>> first match heuristics: 90.00%
>> combined heuristics: 90.00%
>> __builtin_expect heuristics of edge 2->3: 90.00%
>>
>> As seen here __builtin_expect is stronger as it's first match heuristics and
>> has probability == 90%.
>>
>> ;; Function bar (_Z3barv, funcdef_no=1, decl_uid=2303, cgraph_uid=2,
>> symbol_order=4)
>>
>> Predictions for bb 2
>> DS theory heuristics: 74.48%
>> combined heuristics: 74.48%
>> opcode values nonequal (on trees) heuristics of edge 2->3: 34.00%
>> hot label heuristics of edge 2->3: 85.00%
>>
>> Here we combine hot label prediction with the opcode one, resulting in quite
>> poor result 75%.
>> So maybe cold/hot prediction cal also happen first match.
>
> Makes sense.
Good, let me prepare patch for it.
>
>> 2) ./xgcc -B. -O2 -fdump-tree-profile_estimate=/dev/stdout /tmp/test2.C
>> ...
>> foo ()
>> {
>> ...
>> switch (_3) <default: <L3> [3.33%], case 3: <L0> [3.33%], case 42: <L1>
>> [3.33%], case 333: <L2> [90.00%]>
>>
>> while:
>>
>> bar ()
>> {
>> switch (a.1_1) <default: <L3> [25.00%], case 3: <L0> [25.00%], case 42:
>> <L1> [25.00%], case 333: <L2> [25.00%]>
>> ...
>>
>> Note that support for __builtin_expect was enhanced in this stage1. I can
>> definitely cover also situations when one uses
>> hot/cold for labels. So definitely place for improvement.
>
> Hmm, the gimplifier should be adding a PREDICT_EXPR for the case label
> now, is it just ignored currently?
Yes, for switch multi-branches yes. I'll prepare patch for it, should be quite
straightforward.
>
>> 3) last example where one can use the attribute for function decl, resulting
>> in:
>> __attribute__((hot, noinline))
>> foo ()
>> {
>> ..
>>
>> Hope it's desired? If so I would cover that with a test-case in test-suite.
>
> [[likely]] and [[unlikely]] don't apply to functions; I suppose I
> should diagnose that.
Yes.
>
>> Jason can you please point to C++ specification of the attributes?
>
> http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p0479r5.html
>
>> Would you please consider an error diagnostics for situations written in
>> test4.C?
>
> A warning seems appropriate. You think the front end is the right
> place for that?
Probably yes. Note that middle-end can optimize about dead branches and so that
theoretically one can end up with a branching where e.g. both branches are
[[likely]].
I wouldn't bother users with these.
Martin
>
> Jason
>
