It seems like __builtin_expect doesn't propagate to conditions when inlined. This is unfortunate because in some cases it would be nice to be able to put the expectation into methods so that every user doesn't need to do their own hinting. Currently we need to use macros to achieve this. See https://godbolt.org/g/MuPM77 for full example inline bool expect_false(bool b) { return __builtin_expect(b, false); } void inline_func_hint(bool b) { if (expect_false(b)) { unlikely(); // treated as more likely!!! } else { likely(); } } inline_func_hint(bool): test dil, dil je .L11 jmp unlikely() .L11: jmp likely()
LLVM bug: https://bugs.llvm.org/show_bug.cgi?id=37476
I believe inlining is happening too late for it to have an effect - we are early inlining a body which has the __builtin_expect replaced by nothing. Iff the expected outcome is "constant" a new function attribute would work. For outcome dependent on context that wouldn't work. Not sure if we can simply introduce a return-value predictor that survives inlining? Honza?
I can analyze that..
> I believe inlining is happening too late for it to have an effect - we are > early inlining a body which has the __builtin_expect replaced by nothing. > > Iff the expected outcome is "constant" a new function attribute would work. > For outcome dependent on context that wouldn't work. > > Not sure if we can simply introduce a return-value predictor that survives > inlining? Honza? with martin we added strip_predict_hints to the end of early optimization queue at https://gcc.gnu.org/ml/gcc-patches/2017-06/msg01573.html this removes the hint and it is not used by the branch prediction of the outer function. The statement predictors are somewhat problematic here becuase they may associate with wrong conditional if the inner function body is optimized. I guess we have two options 1) do not re-do branch prediction on inlined code. 2) strip just selected hints and not others (like builtin_expect) that is safe even after inlning. Not sure which one is better... Honza > > -- > You are receiving this mail because: > You are on the CC list for the bug.
Well, it would require partial stripping of selected predictors and then later stripping all of them after profile_estimate pass. About the __builtin_expect, I would recommend to use macro, similarly what Linux does: $ cat include/linux/compiler.h ... #define likely_notrace(x) __builtin_expect(!!(x), 1) #define unlikely_notrace(x) __builtin_expect(!!(x), 0) ... Thus lowering priority for the PR.
Using macros is not an acceptable solution for idiomatic C++.
(In reply to Jonathan Wakely from comment #6) > Using macros is not an acceptable solution for idiomatic C++. I see. Then let me really fix that.
So what happens, pass_strip_predict_hints is called as last pass_all_early_optimizations pass. That's called for first function. Then the second one goes through einline pass, but in this time the first one is already stripped. So solution would be to make pass_strip_predict_hints a simple IPA pass and put it right after that. I have patch for that. That would mean that following will not be striped before einline: 1) __builtin_expect (plus corresponding IFN) - it's fine 2) predict_exprs (and gimple_predict_exprs) - PRED_GOTO, PRED_CONTINUE, PRED_FORTRAN_FAIL_IO, PRED_NORETURN, PRED_FORTRAN_WARN_ONCE, PRED_TREE_EARLY_RETURN Hope all are fine, maybe PRED_TREE_EARLY_RETURN is bit unreliable? Honza what do you think? If you agree, I can make it IPA pass. Having that, we'll have: Predictions for bb 2 call heuristics of edge 2->4 (edge pair duplicate): 33.00% call heuristics of edge 2->3 (edge pair duplicate): 33.00% first match heuristics: 10.00% combined heuristics: 10.00% __builtin_expect heuristics of edge 2->3: 10.00% Predictions for bb 3 1 edges in bb 3 predicted to even probabilities Predictions for bb 4 1 edges in bb 4 predicted to even probabilities Predictions for bb 5 1 edges in bb 5 predicted to even probabilities inline_func_hint (bool b) { bool D.2307; bool b; long int _9; long int _10; <bb 2> [local count: 1073741825]: _9 = (long int) b_3(D); _10 = __builtin_expect (_9, 0); if (_10 != 0) goto <bb 3>; [10.00%] else goto <bb 4>; [90.00%] <bb 3> [local count: 107374182]: unlikely (); goto <bb 5>; [100.00%] <bb 4> [local count: 966367642]: likely (); <bb 5> [local count: 1073741825]: return; }
Author: marxin Date: Fri Aug 10 09:31:51 2018 New Revision: 263465 URL: https://gcc.gnu.org/viewcvs?rev=263465&root=gcc&view=rev Log: Strip only selected predictors after early tree passes (PR tree-optimization/85799). 2018-08-10 Martin Liska <mliska@suse.cz> PR tree-optimization/85799 * passes.def: Add argument for pass_strip_predict_hints. * predict.c (class pass_strip_predict_hints): Add new argument early_p. (strip_predictor_early): New function. (pass_strip_predict_hints::execute): Call the function to strip predictors. (strip_predict_hints): New function. * predict.def: Fix comment. 2018-08-10 Martin Liska <mliska@suse.cz> PR tree-optimization/85799 * gcc.dg/pr85799.c: New test. Added: trunk/gcc/testsuite/gcc.dg/pr85799.c Modified: trunk/gcc/ChangeLog trunk/gcc/passes.def trunk/gcc/predict.c trunk/gcc/predict.def trunk/gcc/testsuite/ChangeLog
Fixed on trunk.