diff mbox

Improve probability/profile distribution in ORIF expansion

Message ID CAAe5K+Xy_XthATu5P_YtdTuFRzLwEN-iYfkKuh-9T9qKEP=gYw@mail.gmail.com
State New
Headers show

Commit Message

Teresa Johnson Oct. 1, 2013, 10:48 p.m. UTC
This patch fixes an issue where expansion of an ORIF expression arbitrarily
applied the probability that the entire condition was true to just the
first condition. When the ORIF true probability was 100%, this resulted
in the second condition's jump being given a count of zero (since the
first condition's jump got 100% of the count), leading to incorrect function
splitting when it had a non-zero probability in reality. Since there
currently isn't better information about which condition resulted
in the ORIF being true, apply a 50-50 probability that it is the first
vs. second condition that caused the entire expression to be true,
so that neither condition's true label ends up as a 0-count bb.

Bootstrapped and tested on x86_64-unknown-linux-gnu. Ok for trunk?

2013-10-01  Teresa Johnson  <tejohnson@google.com>

        * dojump.c (do_jump_1): Divide probability between
        both conditions of a TRUTH_ORIF_EXPR.

       gcc_unreachable ();

Comments

Jan Hubicka Oct. 2, 2013, 4:03 p.m. UTC | #1
> 2013-10-01  Teresa Johnson  <tejohnson@google.com>
> 
>         * dojump.c (do_jump_1): Divide probability between
>         both conditions of a TRUTH_ORIF_EXPR.
> 
> +      {
> +        /* Spread the probability evenly between the two conditions. So
> +           the first condition has half the total probability of being true.
> +           The second condition has the other half of the total probability,
> +           so its jump has a probability of half the total, relative to
> +           the probability we reached it (i.e. the first condition
> was false).  */
> +        int op0_prob = prob / 2;
> +        int op1_prob = GCOV_COMPUTE_SCALE ((prob / 2), inv (op0_prob));

Documentation of the functions says that PROB may be -1 when it is unknown, In that
case you want to arrange op0_prob=op1_prob = -1.

What about TRUTH_ANDIF_EXPR code above?  I think it needs similar adjusting

Patch is preaproved with these changes.

Thanks!
Honza
diff mbox

Patch

Index: dojump.c
===================================================================
--- dojump.c    (revision 203077)
+++ dojump.c    (working copy)
@@ -325,18 +325,27 @@  do_jump_1 (enum tree_code code, tree op0, tree op1
       break;

     case TRUTH_ORIF_EXPR:
-      if (if_true_label == NULL_RTX)
-       {
-          drop_through_label = gen_label_rtx ();
-         do_jump (op0, NULL_RTX, drop_through_label, prob);
-         do_jump (op1, if_false_label, NULL_RTX, prob);
-       }
-      else
-       {
-         do_jump (op0, NULL_RTX, if_true_label, prob);
-         do_jump (op1, if_false_label, if_true_label, prob);
-       }
-      break;
+      {
+        /* Spread the probability evenly between the two conditions. So
+           the first condition has half the total probability of being true.
+           The second condition has the other half of the total probability,
+           so its jump has a probability of half the total, relative to
+           the probability we reached it (i.e. the first condition
was false).  */
+        int op0_prob = prob / 2;
+        int op1_prob = GCOV_COMPUTE_SCALE ((prob / 2), inv (op0_prob));
+        if (if_true_label == NULL_RTX)
+          {
+            drop_through_label = gen_label_rtx ();
+            do_jump (op0, NULL_RTX, drop_through_label, op0_prob);
+            do_jump (op1, if_false_label, NULL_RTX, op1_prob);
+          }
+        else
+          {
+            do_jump (op0, NULL_RTX, if_true_label, op0_prob);
+            do_jump (op1, if_false_label, if_true_label, op1_prob);
+          }
+        break;
+      }

     default: