This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
[PATCH] Fix up cmove expansion (PR target/58864)
- From: Jakub Jelinek <jakub at redhat dot com>
- To: Eric Botcazou <ebotcazou at adacore dot com>, Steven Bosscher <stevenb dot gcc at gmail dot com>, Richard Biener <rguenther at suse dot de>
- Cc: gcc-patches at gcc dot gnu dot org
- Date: Fri, 29 Nov 2013 22:56:30 +0100
- Subject: [PATCH] Fix up cmove expansion (PR target/58864)
- Authentication-results: sourceware.org; auth=none
- Reply-to: Jakub Jelinek <jakub at redhat dot com>
Hi!
The following testcase ICEs because expand_cond_expr_using_cmove
calls emit_conditional_move (which calls do_pending_stack_adjust
under some circumstances), but when that fails, just removes all the
insns generated by emit_conditional_move (and perhaps some earlier ones
too), thus it removes also the stack adjustment.
Apparently 2 similar places were fixing it by just calling
do_pending_stack_adjust () first just in case, some other places
had (most likely) the same bug as this function.
Rather than adding do_pending_stack_adjust () in all the places, especially
when it isn't clear whether emit_conditional_move will be called at all and
whether it will actually do do_pending_stack_adjust (), I chose to add
two new functions to save/restore the pending stack adjustment state,
so that when instruction sequence is thrown away (either by doing
start_sequence/end_sequence around it and not emitting it, or
delete_insns_since) the state can be restored, and have changed all the
places that IMHO need it for emit_conditional_move.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk/4.8?
2013-11-29 Jakub Jelinek <jakub@redhat.com>
PR target/58864
* dojump.c (save_pending_stack_adjust, restore_pending_stack_adjust):
New functions.
* expr.h (save_pending_stack_adjust, restore_pending_stack_adjust):
New prototypes.
* expr.c (expand_cond_expr_using_cmove): Use it.
(expand_expr_real_2): Use it instead of unconditional
do_pending_stack_adjust.
* optabs.c (expand_doubleword_shift): Use it.
* expmed.c (expand_sdiv_pow2): Use it instead of unconditional
do_pending_stack_adjust.
(emit_store_flag): Use it.
* g++.dg/opt/pr58864.C: New test.
--- gcc/expr.c.jj 2013-11-27 18:02:46.000000000 +0100
+++ gcc/expr.c 2013-11-29 14:35:12.234808484 +0100
@@ -7951,6 +7951,9 @@ expand_cond_expr_using_cmove (tree treeo
else
temp = assign_temp (type, 0, 1);
+ int save[2];
+ save_pending_stack_adjust (save);
+
start_sequence ();
expand_operands (treeop1, treeop2,
temp, &op1, &op2, EXPAND_NORMAL);
@@ -8009,6 +8012,7 @@ expand_cond_expr_using_cmove (tree treeo
/* Otherwise discard the sequence and fall back to code with
branches. */
end_sequence ();
+ restore_pending_stack_adjust (save);
#endif
return NULL_RTX;
}
@@ -8789,12 +8793,9 @@ expand_expr_real_2 (sepops ops, rtx targ
if (can_conditionally_move_p (mode))
{
rtx insn;
+ int save[2];
- /* ??? Same problem as in expmed.c: emit_conditional_move
- forces a stack adjustment via compare_from_rtx, and we
- lose the stack adjustment if the sequence we are about
- to create is discarded. */
- do_pending_stack_adjust ();
+ save_pending_stack_adjust (save);
start_sequence ();
@@ -8817,6 +8818,7 @@ expand_expr_real_2 (sepops ops, rtx targ
/* Otherwise discard the sequence and fall back to code with
branches. */
end_sequence ();
+ restore_pending_stack_adjust (save);
}
#endif
if (target != op0)
--- gcc/optabs.c.jj 2013-11-19 21:56:22.000000000 +0100
+++ gcc/optabs.c 2013-11-29 14:39:15.963513835 +0100
@@ -1079,17 +1079,20 @@ expand_doubleword_shift (enum machine_mo
#ifdef HAVE_conditional_move
/* Try using conditional moves to generate straight-line code. */
- {
- rtx start = get_last_insn ();
- if (expand_doubleword_shift_condmove (op1_mode, binoptab,
- cmp_code, cmp1, cmp2,
- outof_input, into_input,
- op1, superword_op1,
- outof_target, into_target,
- unsignedp, methods, shift_mask))
- return true;
- delete_insns_since (start);
- }
+ int save[2];
+
+ save_pending_stack_adjust (save);
+
+ rtx start = get_last_insn ();
+ if (expand_doubleword_shift_condmove (op1_mode, binoptab,
+ cmp_code, cmp1, cmp2,
+ outof_input, into_input,
+ op1, superword_op1,
+ outof_target, into_target,
+ unsignedp, methods, shift_mask))
+ return true;
+ delete_insns_since (start);
+ restore_pending_stack_adjust (save);
#endif
/* As a last resort, use branches to select the correct alternative. */
--- gcc/dojump.c.jj 2013-11-19 21:56:27.000000000 +0100
+++ gcc/dojump.c 2013-11-29 14:35:35.088685749 +0100
@@ -96,6 +96,29 @@ do_pending_stack_adjust (void)
pending_stack_adjust = 0;
}
}
+
+/* Remember pending_stack_adjust/stack_pointer_delta.
+ To be used around code that may call do_pending_stack_adjust (),
+ but the generated code could be discarded e.g. using delete_insns_since. */
+
+void
+save_pending_stack_adjust (int save[2])
+{
+ save[0] = pending_stack_adjust;
+ save[1] = stack_pointer_delta;
+}
+
+/* Restore the saved pending_stack_adjust/stack_pointer_delta. */
+
+void
+restore_pending_stack_adjust (int save[2])
+{
+ if (inhibit_defer_pop == 0)
+ {
+ pending_stack_adjust = save[0];
+ stack_pointer_delta = save[1];
+ }
+}
/* Expand conditional expressions. */
--- gcc/expr.h.jj 2013-11-19 21:56:27.000000000 +0100
+++ gcc/expr.h 2013-11-29 14:35:23.438747999 +0100
@@ -473,6 +473,16 @@ extern void clear_pending_stack_adjust (
/* Pop any previously-pushed arguments that have not been popped yet. */
extern void do_pending_stack_adjust (void);
+/* Remember pending_stack_adjust/stack_pointer_delta.
+ To be used around code that may call do_pending_stack_adjust (),
+ but the generated code could be discarded e.g. using delete_insns_since. */
+
+extern void save_pending_stack_adjust (int [2]);
+
+/* Restore the saved pending_stack_adjust/stack_pointer_delta. */
+
+extern void restore_pending_stack_adjust (int [2]);
+
/* Return the tree node and offset if a given argument corresponds to
a string constant. */
extern tree string_constant (tree, tree *);
--- gcc/expmed.c.jj 2013-11-19 21:56:36.000000000 +0100
+++ gcc/expmed.c 2013-11-29 14:41:16.887872205 +0100
@@ -3735,11 +3735,9 @@ expand_sdiv_pow2 (enum machine_mode mode
>= 2)
{
rtx temp2;
+ int save[2];
- /* ??? emit_conditional_move forces a stack adjustment via
- compare_from_rtx so, if the sequence is discarded, it will
- be lost. Do it now instead. */
- do_pending_stack_adjust ();
+ save_pending_stack_adjust (save);
start_sequence ();
temp2 = copy_to_mode_reg (mode, op0);
@@ -3758,6 +3756,7 @@ expand_sdiv_pow2 (enum machine_mode mode
return expand_shift (RSHIFT_EXPR, mode, temp2, logd, NULL_RTX, 0);
}
end_sequence ();
+ restore_pending_stack_adjust (save);
}
#endif
@@ -5508,6 +5507,9 @@ emit_store_flag (rtx target, enum rtx_co
if (tem == 0)
return 0;
+ int save[2];
+ save_pending_stack_adjust (save);
+
if (and_them)
tem = emit_conditional_move (target, code, op0, op1, mode,
tem, const0_rtx, GET_MODE (tem), 0);
@@ -5516,7 +5518,10 @@ emit_store_flag (rtx target, enum rtx_co
trueval, tem, GET_MODE (tem), 0);
if (tem == 0)
- delete_insns_since (last);
+ {
+ delete_insns_since (last);
+ restore_pending_stack_adjust (save);
+ }
return tem;
#else
return 0;
--- gcc/testsuite/g++.dg/opt/pr58864.C.jj 2013-11-29 13:49:24.450415660 +0100
+++ gcc/testsuite/g++.dg/opt/pr58864.C 2013-11-29 13:47:47.000000000 +0100
@@ -0,0 +1,21 @@
+// PR target/58864
+// { dg-do compile }
+// { dg-options "-Os" }
+// { dg-additional-options "-march=i686" { target { { i?86-*-* x86_64-*-* } && ia32 } } }
+
+struct A { A (); ~A (); };
+struct B { B (); };
+
+float d, e;
+
+void
+foo ()
+{
+ A a;
+ float c = d;
+ while (1)
+ {
+ B b;
+ e = c ? -c : 0;
+ }
+}
Jakub