Dne 2018-01-22 16:57, Jakub Jelinek napsal:
On Mon, Jan 22, 2018 at 02:43:50PM +0100, Jan Hubicka wrote:
> But this looks incorrect, because that computes only the above c2 in
> prob, not
> second_prob. It needs to be
> prob = cprob.invert () * prob / first_prob.invert ();
> or
> prob *= cprob.invert () / first_prob.invert ();
> or that
> prob = (prob - first_prob) / first_prob.invert ();
> I had in the patch. The last one looked to me like cheapest to compute.
Aha, I see. Ok that makes sense to me now :)
>
> > void profile_probability::split (profile_probability cprob,
> > profile_probability *first_prob, profile_probability *second_prob)
>
> This doesn't take prob as an argument, or should it be a method and take
> prob as *this? Couldn't it simply return first_prob and adjust itself,
> so
> profile_probability split (profile_probability cprob)
> {
> profile_probability ret = *this * cprob;
> *this = (*this - prob) / ret.invert ();
> // or *this = *this * cprob.invert () / ret.invert ();
> return ret;
> }
> ?
Yep, that is fine.
So like this if it passes bootstrap/regtest on {x86_64,i686}-linux?
Besides adding the new helper method and using it in the spots I've
changed
plus the two you've mentioned, I had to do another fix - looking
at how TRUTH_ANDIF_EXPR is handled in dojump_1 and seeing Invalid sum
messages in predict-8.c's *.expand jump, I've realized that it also
works
as is only for the discussed, i.e.
if (x)
goto t; // prob
transformed into:
if (a)
goto t; // first_prob
if (b)
goto t; // prob
but not for the case where and_them is true, where we have:
if (c)
goto dummy; // first_prob.invert ()
if (d)
goto t; // prob
dummy:
which needs to be handled similarly to the TRUTH_ANDIF_EXPR case.
At least when trying in gdb on predict-8.c various values of cprob
(1%, 15%, 50%, 62%, 98%) none of them generate the Invalid sum
messages.
2018-01-22 Jakub Jelinek <ja...@redhat.com>
PR tree-optimization/83081
* profile-count.h (profile_probability::split): New method.
* dojump.c (do_jump_1) <case TRUTH_ANDIF_EXPR, case TRUTH_ORIF_EXPR>:
Use profile_probability::split.
(do_compare_rtx_and_jump): Fix adjustment of probabilities
when splitting a single conditional jump into 2.
* gcc.dg/predict-8.c: Adjust expected probability.
--- gcc/profile-count.h.jj 2018-01-19 19:41:52.910549618 +0100
+++ gcc/profile-count.h 2018-01-22 16:20:27.073096515 +0100
@@ -410,6 +410,19 @@ public:
return *this;
}
+ /* Split *THIS (ORIG) probability into 2 probabilities, such that
+ the returned one (FIRST) is *THIS * CPROB and *THIS is
+ adjusted (SECOND) so that FIRST + FIRST.invert () * SECOND
+ == ORIG. */
To make documentation clear, i would include the pseudocode with
conditionals and individual
probabilities.
It is bit non-obvious transformation and it would be nice to reduce
level of apparent
magicness in profiling code :)
OK,
Honza
+ profile_probability split (const profile_probability &cprob)
+ {
+ profile_probability ret = *this * cprob;
+ /* The following is equivalent to:
+ *this = cprob.invert () * *this / ret.invert (); */
+ *this = (*this - ret) / ret.invert ();
+ return ret;
+ }
+
gcov_type apply (gcov_type val) const
{
if (*this == profile_probability::uninitialized ())
--- gcc/dojump.c.jj 2018-01-19 19:41:49.978548984 +0100
+++ gcc/dojump.c 2018-01-22 16:31:43.867185428 +0100
@@ -347,13 +347,11 @@ do_jump_1 (enum tree_code code, tree op0
profile_probability op1_prob =
profile_probability::uninitialized ();
if (prob.initialized_p ())
{
- profile_probability false_prob = prob.invert ();
- profile_probability op0_false_prob =
false_prob.apply_scale (1, 2);
- profile_probability op1_false_prob = false_prob.apply_scale (1,
2)
- / op0_false_prob.invert ();
+ op1_prob = prob.invert ();
+ op0_prob = op1_prob.split (profile_probability::even ());
/* Get the probability that each jump below is true. */
- op0_prob = op0_false_prob.invert ();
- op1_prob = op1_false_prob.invert ();
+ op0_prob = op0_prob.invert ();
+ op1_prob = op1_prob.invert ();
}
if (if_false_label == NULL)
{
@@ -380,8 +378,8 @@ do_jump_1 (enum tree_code code, tree op0
profile_probability op1_prob =
profile_probability::uninitialized ();
if (prob.initialized_p ())
{
- op0_prob = prob.apply_scale (1, 2);
- op1_prob = prob.apply_scale (1, 2) / op0_prob.invert ();
+ op1_prob = prob;
+ op0_prob = op1_prob.split (profile_probability::even ());
}
if (if_true_label == NULL)
{
@@ -1120,16 +1118,27 @@ do_compare_rtx_and_jump (rtx op0, rtx op
else
{
- profile_probability first_prob = prob;
+ profile_probability cprob
+ = profile_probability::guessed_always ();
if (first_code == UNORDERED)
- first_prob = profile_probability::guessed_always ().apply_scale
- (1, 100);
+ cprob = cprob.apply_scale (1, 100);
else if (first_code == ORDERED)
- first_prob = profile_probability::guessed_always ().apply_scale
- (99, 100);
+ cprob = cprob.apply_scale (99, 100);
+ else
+ cprob = profile_probability::even ();
+ /* We want to split:
+ if (x) goto t; // prob;
+ into
+ if (a) goto t; // first_prob;
+ if (b) goto t; // prob;
+ such that the overall probability of jumping to t
+ remains the same and first_prob is prob * cprob. */
if (and_them)
{
rtx_code_label *dest_label;
+ prob = prob.invert ();
+ profile_probability first_prob = prob.split (cprob).invert ();
+ prob = prob.invert ();
/* If we only jump if true, just bypass the second jump. */
if (! if_false_label)
{
@@ -1143,8 +1152,11 @@ do_compare_rtx_and_jump (rtx op0, rtx op
size, dest_label, NULL, first_prob);
}
else
- do_compare_rtx_and_jump (op0, op1, first_code,
unsignedp, mode,
- size, NULL, if_true_label, first_prob);
+ {
+ profile_probability first_prob = prob.split (cprob);
+ do_compare_rtx_and_jump (op0, op1, first_code, unsignedp,
mode,
+ size, NULL, if_true_label,
first_prob);
+ }
}
}
--- gcc/testsuite/gcc.dg/predict-8.c.jj 2017-10-12 20:51:33.192974887
+0200
+++ gcc/testsuite/gcc.dg/predict-8.c 2018-01-22 16:18:09.528071106
+0100
@@ -8,4 +8,4 @@ int foo(float a, float b) {
return 2;
}
-/* { dg-final { scan-rtl-dump-times "99.0. .guessed" 2 "expand"} } */
+/* { dg-final { scan-rtl-dump-times "65.\[34]. .guessed" 2 "expand"} }
*/
Jakub
