[PATCH] (Partial) fix for PR middle-end/13392

[Jakub Jelinek <jakub at redhat dot com>]

Hi!

This is only partial fix, fixes the testcase and many others but doesn't
actually cure the real bug.  But, it should also improve generated code.
Current expand_builtin_expect_jump groks cases where it finds:
cond_jump -> if_true_label
or
cond_jump -> if_false_label
but fails to SCC if it finds:
cond_jump -> drop_through_label
uncond_jump -> if_true_label (or if_false_label)
barrier
drop_through_label:

This patch cures that.  Also, for
if (__builtin_expect (foo () && bar (), 1))
  goto if_true_label;
which could expand to e.g.
foo ()
cond_jump -> drop_through_label
bar ()
cond_jump -> if_true_label
drop_through_label:
without this patch it sets probability only for the second conditional jump,
while IMHO it should set it for both (PROB_VERY_LIKELY for the second
cond_jump and PROB_VERY_UNLIKELY for the first one).

Now, the real bug is that if do_jump in expand_builtin_expect_jump
doesn't emit equal number of NOTE_INSN_EH_REGION_BEG and
NOTE_INSN_EH_REGION_END notes and expand_builtin_expect_jump decides to fall
back into SCC, EH will be inconsistent as some notes will be discarded while
the matching ones emitted.  Either unsafe_for_reeval should return 2 in such
cases, or expand_builtin_expect_jump should check whether it is in the same
EH region before do_jump call and after it.  In the latter case it could
either call some function which would tell expect.c that insn sequence is
going away, or it could avoid falling back to SCC, even if no branch
prediction has been set.

Another thing which could be done is to handle !flag_guess_branch_prob
specially in expand_builtin_expect_jump, particularly never fall back
to SCC in that case and don't check unsafe_for_reeval, as predict_insn*
is a nop when flag_guess_branch_prob is 0.

Ok to commit this patch? 3.3 as well?
What do you suggest doing about the unbalanced EH problem?
Do you think !flag_guess_branch_prob is worth special casing?

2003-12-31  Jakub Jelinek  <jakub@redhat.com>

	PR middle-end/13392
	* builtins.c (expand_builtin_expect_jump): Handle conditional jumps
	to drop through label.

	* g++.dg/opt/expect2.C: New test.

--- gcc/builtins.c.jj	2003-12-19 14:17:23.000000000 +0100
+++ gcc/builtins.c	2003-12-31 18:00:03.000000000 +0100
@@ -4434,9 +4434,10 @@ expand_builtin_expect_jump (tree exp, rt
   if (TREE_CODE (TREE_TYPE (arg1)) == INTEGER_TYPE
       && (integer_zerop (arg1) || integer_onep (arg1)))
     {
-      int num_jumps = 0;
+      int num_condjumps = 0, num_dropthrough_jumps = 0;
+      int num_uncondjumps = 0;
       int save_pending_stack_adjust = pending_stack_adjust;
-      rtx insn;
+      rtx insn, drop_through_label;
 
       /* If we fail to locate an appropriate conditional jump, we'll
 	 fall back to normal evaluation.  Ensure that the expression
@@ -4458,8 +4459,19 @@ expand_builtin_expect_jump (tree exp, rt
       start_sequence ();
       do_jump (arg0, if_false_label, if_true_label);
       ret = get_insns ();
+
+      drop_through_label = get_last_insn ();
+      if (drop_through_label && GET_CODE (drop_through_label) == NOTE)
+	drop_through_label = prev_nonnote_insn (drop_through_label);
+      if (drop_through_label && GET_CODE (drop_through_label) != CODE_LABEL)
+	drop_through_label = NULL_RTX;
       end_sequence ();
 
+      if (! if_true_label)
+	if_true_label = drop_through_label;
+      if (! if_false_label)
+	if_false_label = drop_through_label;
+
       /* For mildly unsafe builtin jump's, if unsave_expr_now
 	 creates a new tree instead of changing the old one
 	 TREE_VALUE (arglist) needs to be updated.  */
@@ -4524,9 +4536,21 @@ expand_builtin_expect_jump (tree exp, rt
 	      else if (label != if_true_label)
 		goto do_next_insn;
 
-	      num_jumps++;
+	      if (label == drop_through_label)
+		num_dropthrough_jumps++;
+	      else
+		num_condjumps++;
 	      predict_insn_def (insn, PRED_BUILTIN_EXPECT, taken);
 	    }
+	  else if (GET_CODE (insn) == JUMP_INSN && any_uncondjump_p (insn))
+	    {
+	      rtx set_pc = pc_set (insn);
+	      rtx label = XEXP (SET_SRC (set_pc), 0);
+
+	      if ((label == if_false_label || label == if_true_label)
+		  && label != drop_through_label)
+		num_uncondjumps++;
+	    }
 
 	do_next_insn:
 	  insn = next;
@@ -4534,7 +4558,8 @@ expand_builtin_expect_jump (tree exp, rt
 
       /* If no jumps were modified, fail and do __builtin_expect the normal
 	 way.  */
-      if (num_jumps == 0)
+      if (num_condjumps == 0
+	  && (num_dropthrough_jumps == 0 || num_uncondjumps == 0))
 	{
 	  ret = NULL_RTX;
 	  pending_stack_adjust = save_pending_stack_adjust;
--- gcc/testsuite/g++.dg/opt/expect2.C.jj	2003-12-31 18:14:54.000000000 +0100
+++ gcc/testsuite/g++.dg/opt/expect2.C	2003-12-31 18:21:30.000000000 +0100
@@ -0,0 +1,11 @@
+// PR middle-end/13392
+// { dg-do run }
+// { dg-options "-O0" }
+
+extern "C" void abort (void);
+struct X { ~X () throw() {} };
+bool foo (X s = X ()) { return false; }
+void bar ()
+{
+  __builtin_expect (foo () && true, 1) ? 0 : (abort (), 0);
+}

	Jakub