This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH] Fix up cmove expansion (PR target/58864)


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]