[PATCH] Fix __builtin_expect (setjmp (buf) == 0, 1)) generates broken code
Jakub Jelinek
jakub@redhat.com
Thu Mar 20 13:12:00 GMT 2003
On Thu, Mar 20, 2003 at 12:20:20PM +0100, Jakub Jelinek wrote:
> I agree with Zack here. __builtin_expect is not part of the C99 standard
> and thus it is up to us how we define it. And IMHO it should just set
> branch probablity, nothing else.
>
> FYI, while:
> if (__builtin_expect (setjmp (b) == 0, 1))
> bar ();
> doesn't work,
> if (__builtin_expect (setjmp (b), 0) == 0)
> bar ();
> seems to work just fine, which leads to another thing:
>
> int baz (void);
> void bar (void);
> void foo (void)
> {
> if (__builtin_expect (baz (), 0) == 0)
> bar ();
> }
>
> vs.
>
> int baz (void);
> void bar (void);
> void foo (void)
> {
> if (__builtin_expect (baz () == 0, 1))
> bar ();
> }
>
> (this time no messing with functions returning twice etc.), generates
> the exact same assembly when optimizing on IA-32, x86-64, IA-64, sparc -m32,
> but generates worse code for the second variant on s390, s390x, sparc -m64.
>
> When __builtin_expect makes the code worse rather then better is somewhat
> disturbing (but the first form really cannot be used everywhere, it requires
> that first builtin expect argument is integral type and the expected
> return value is constant).
Fixed thusly:
The problem seems to be that unsafe_for_reeval returned 1 for baz () == 0
and thus baz () == 0 was surrounded into UNSAVE_EXPR. But do_jump didn't
handle UNSAVE_EXPR specially, so it did expand_expr on the UNSAVE_EXPR
forcing baz () == 0 into a temporary and then did a conditional jummp based
on that.
Ok to commit if testing succeeds? What about 3.2/3.3 (with the obvious
changes (expr.c instead of dojump.c in both and unsave_expr_now instead
of (*lang_hooks.unsave_expr_now) on 3.2))?
2003-03-20 Jakub Jelinek <jakub@redhat.com>
* dojump.c (do_jump): Handle UNSAVE_EXPR specially.
--- gcc/dojump.c.jj 2003-03-13 11:05:23.000000000 -0500
+++ gcc/dojump.c 2003-03-20 07:52:49.000000000 -0500
@@ -160,6 +160,12 @@ do_jump (exp, if_false_label, if_true_la
break;
#endif
+ case UNSAVE_EXPR:
+ do_jump (TREE_OPERAND (exp, 0), if_false_label, if_true_label);
+ TREE_OPERAND (exp, 0)
+ = (*lang_hooks.unsave_expr_now) (TREE_OPERAND (exp, 0));
+ break;
+
case NOP_EXPR:
if (TREE_CODE (TREE_OPERAND (exp, 0)) == COMPONENT_REF
|| TREE_CODE (TREE_OPERAND (exp, 0)) == BIT_FIELD_REF
Jakub
More information about the Gcc-patches
mailing list