dojump: Fix up probabilities splitting in dojump.c comparison splitting [PR98212]
authorJakub Jelinek <jakub@redhat.com>
Thu, 10 Dec 2020 23:36:21 +0000 (00:36 +0100)
committerJakub Jelinek <jakub@redhat.com>
Thu, 10 Dec 2020 23:36:21 +0000 (00:36 +0100)
When compiling:
void foo (void);
void bar (float a, float b) { if (__builtin_expect (a != b, 1)) foo (); }
void baz (float a, float b) { if (__builtin_expect (a == b, 1)) foo (); }
void qux (float a, float b) { if (__builtin_expect (a != b, 0)) foo (); }
void corge (float a, float b) { if (__builtin_expect (a == b, 0)) foo (); }
on x86_64, we get (unimportant cruft removed):
bar:    ucomiss %xmm1, %xmm0
        jp      .L4
        je      .L1
.L4:    jmp     foo
.L1:    ret
baz:    ucomiss %xmm1, %xmm0
        jp      .L6
        jne     .L6
        jmp     foo
.L6:    ret
qux:    ucomiss %xmm1, %xmm0
        jp      .L13
        jne     .L13
        ret
.L13:   jmp     foo
corge:  ucomiss %xmm1, %xmm0
        jnp     .L18
.L14:   ret
.L18:   jne     .L14
        jmp     foo
(note for bar and qux that changed with a patch I've posted earlier today).
This is all reasonable, except the last function, the overall jump to
the tail call is predicted unlikely (10%), so it is good jmp foo isn't on
the straight line path, but NaNs are (or should be) considered very unlikely
in the programs, so IMHO the right code (and one emitted with the following
patch) is:
corge:  ucomiss %xmm1, %xmm0
        jp      .L14
        je      .L18
.L14:   ret
.L18:   jmp     foo

Let's discuss the probabilities in the above testcase:
for !and_them it looks all correct, so for
bar we split
if (a != b) goto t; // prob 90%
goto f;
into:
if (a unord b) goto t; // first_prob = prob * cprob = 90% * 1% = 0.9%
if (a ltgt b) goto t; // adjusted prob = (prob - first_prob) / (1 - first_prob) = (90% - 0.9%) / (1 - 0.9%) = 89.909%
and for qux we split
if (a != b) goto t; // prob 10%
goto f;
into:
if (a unord b) goto t; // first_prob = prob * cprob = 10% * 1% = 0.1%
if (a ltgt b) goto t; // adjusted prob = (prob - first_prob) / (1 - first_prob) = (10% - 0.1%) / (1 - 0.1%) = 9.910%
Now, the and_them cases should be probability wise exactly the same
if we swap the f and t labels, because baz
if (a == b) goto t; // prob 90%
goto f;
is equivalent to:
if (a != b) goto f; // prob 10%
goto t;
which is in qux.  This means we could expand baz as:
if (a unord b) goto f; // 0.1%
if (a ltgt b) goto f; // 9.910%
goto t;
But we don't expand it exactly that way, but instead (as the comment says)
as:
if (a ord b) ; else goto f; // first_prob as probability of ;
if (a uneq b) goto t; // adjusted prob
goto f;
So, first_prob.invert () should be 0.1% and adjusted prob should be
1 - 9.910%.
Thus, the right thing is 4 inverts:
prob = prob.invert (); // baz is equivalent to qux with swap(t, f) and thus inverted original prob
first_prob = prob.split (cprob.invert ()).invert ();
// cprob.invert because by doing if (cond) ; else goto f; we effectively invert the condition
// the second invert because first_prob is probability of ; rather than goto f
prob = prob.invert (); // lastly because adjusted prob we want is
// probability of goto t;, while the one from corresponding !and_them case
// would be if (...) goto f; goto t;

2020-12-11  Jakub Jelinek  <jakub@redhat.com>

PR rtl-optimization/98212
* dojump.c (do_compare_rtx_and_jump): Change computation of
first_prob for and_them.  Add comment explaining and_them case.

* gcc.dg/predict-8.c: Adjust expected probability.

gcc/dojump.c
gcc/testsuite/gcc.dg/predict-8.c

index b12bcea..09c2b52 100644 (file)
@@ -1138,18 +1138,37 @@ do_compare_rtx_and_jump (rtx op0, rtx op1, enum rtx_code code, int unsignedp,
                cprob = cprob.apply_scale (99, 100);
              else
                cprob = profile_probability::even ();
-             /* We want to split:
+             /* For and_them we want to split:
                 if (x) goto t; // prob;
+                goto f;
                 into
-                if (a) goto t; // first_prob;
-                if (b) goto t; // prob;
+                if (a) ; else goto f; // first_prob for ;
+                                      // 1 - first_prob for goto f;
+                if (b) goto t; // adjusted prob;
+                goto f;
                 such that the overall probability of jumping to t
-                remains the same and first_prob is prob * cprob.  */
+                remains the same.  The and_them case should be
+                probability-wise equivalent to the !and_them case with
+                f and t swapped and also the conditions inverted, i.e.
+                if (!a) goto f;
+                if (!b) goto f;
+                goto t;
+                where the overall probability of jumping to f is
+                1 - prob (thus the first prob.invert () below).
+                cprob.invert () is because the a condition is inverted,
+                so if it was originally ORDERED, !a is UNORDERED and
+                thus should be relative 1% rather than 99%.
+                The invert () on assignment to first_prob is because
+                first_prob represents the probability of fallthru,
+                rather than goto f.  And the last prob.invert () is
+                because the adjusted prob represents the probability of
+                jumping to t rather than to f.  */
              if (and_them)
                {
                  rtx_code_label *dest_label;
                  prob = prob.invert ();
-                 profile_probability first_prob = prob.split (cprob).invert ();
+                 profile_probability first_prob
+                   = prob.split (cprob.invert ()).invert ();
                  prob = prob.invert ();
                  /* If we only jump if true, just bypass the second jump.  */
                  if (! if_false_label)
@@ -1163,6 +1182,15 @@ do_compare_rtx_and_jump (rtx op0, rtx op1, enum rtx_code code, int unsignedp,
                   do_compare_rtx_and_jump (op0, op1, first_code, unsignedp, mode,
                                           size, dest_label, NULL, first_prob);
                }
+             /* For !and_them we want to split:
+                if (x) goto t; // prob;
+                goto f;
+                into
+                if (a) goto t; // first_prob;
+                if (b) goto t; // adjusted prob;
+                goto f;
+                such that the overall probability of jumping to t
+                remains the same and first_prob is prob * cprob.  */
               else
                {
                  profile_probability first_prob = prob.split (cprob);
index 5578175..ec755e2 100644 (file)
@@ -8,4 +8,4 @@ int foo(float a, float b) {
     return 2;
 }
 
-/* { dg-final { scan-rtl-dump-times "65.\[34]. .guessed" 2 "expand"} } */
+/* { dg-final { scan-rtl-dump-times "99.\[678]. .guessed" 2 "expand"} } */