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]

Re: [RFC] PR/27390, latent complex-mode bug in regstack


Paolo Bonzini wrote:
This patch fixes two problems in regstack involving complex modes.

The problem is latent on 4.1 too, and manifests as a stack underflow in gcc.target/x86_64/abi/test_complex_returning.c at -O0. Unluckily, after the PR/19653 patch something convinces reload to produce slightly different RTL, and the stack underflow turns into a wrong-code generation.

The problematic RTL is a clobber in complex mode. When regstack encounters one, it calls move_nan_to_stack_reg twice on the same insn. However, the second call does *not* add a new insn, so we get only one stack load instead of two.

Then, however, when popping a complex floating-point value, the real part is popped first and the imaginary part is popped second. However, this seems to produce wrong code in this case. For this part, I would appreciate help because I am not sure whether what I should reorder these two, or rather I should invert the two pushes in subst_stack_regs_pat.

So, I propose two patches: the first one does not change the move_nan_to_stack_reg order, and only makes sure that both are emitted in the insn stream, and reorders the pops in emit_pop_insn. This patch was bootstrapped/regtested on x86_64-pc-linux-gnu and fixes the regression without introducing any new failure.

The second one changes the move_nan_to_stack_reg order, and of course makes sure that both are emitted in the insn stream. This seems to fix the regression too. It was not bootstrapped/regtested because I do not have access to the affected hardware; Richard Guenther has been very kind in testing the patches for this regression, but I'd rather have counsel from the reg-stack expert before asking him for a third bootstrap/regtest.

Does any of these sound ok, or is either approach completely wrong? (I am not asking explicitly for approval of either patch, but would not be unhappy with it).

ENOPATCH^2. Sorry.


Paolo
2006-06-05  Paolo Bonzini  <bonzini@gnu.org>

	PR target/27390
	* reg-stack.c (subst_stack_regs_pat): Reorder resetting of
	the imaginary and real parts of a clobbered register.
	Emit insn to set the imaginary part.

Index: reg-stack.c
===================================================================
--- reg-stack.c	(revision 113881)
+++ reg-stack.c	(working copy)
@@ -1369,16 +1369,20 @@ subst_stack_regs_pat (rtx insn, stack re
 		if (!note)
 		  {
 		    rtx t = *dest;
-		    if (get_hard_regnum (regstack, t) == -1)
-		      control_flow_insn_deleted
-			|= move_nan_for_stack_reg (insn, regstack, t);
 		    if (COMPLEX_MODE_P (GET_MODE (t)))
 		      {
-			t = FP_MODE_REG (REGNO (t) + 1, DFmode);
-			if (get_hard_regnum (regstack, t) == -1)
-			  control_flow_insn_deleted
-			    |= move_nan_for_stack_reg (insn, regstack, t);
+			rtx u = FP_MODE_REG (REGNO (t) + 1, SFmode);
+			if (get_hard_regnum (regstack, u) == -1)
+			  {
+			    rtx pat2 = gen_rtx_CLOBBER (VOIDmode, u);
+			    rtx insn2 = emit_insn_before (pat2, insn);
+			    control_flow_insn_deleted
+			      |= move_nan_for_stack_reg (insn2, regstack, u);
+			  }
 		      }
+		    if (get_hard_regnum (regstack, t) == -1)
+		      control_flow_insn_deleted
+			|= move_nan_for_stack_reg (insn, regstack, t);
 		  }
 	      }
 	  }
2006-06-05  Paolo Bonzini  <bonzini@gnu,org>

	PR target/27390
        * reg-stack.c (emit_pop_insn): Reorder popping of
        the imaginary and real parts of a clobbered register.
	(subst_stack_regs_pat): Emit insn to set the imaginary part.

Index: ../../gcc/reg-stack.c
===================================================================
--- ../../gcc/reg-stack.c	(revision 113881)
+++ ../../gcc/reg-stack.c	(working copy)
@@ -764,10 +764,10 @@ emit_pop_insn (rtx insn, stack regstack,
       rtx reg2 = FP_MODE_REG (REGNO (reg) + 1, DFmode);
 
       pop_insn = NULL_RTX;
-      if (get_hard_regnum (regstack, reg1) >= 0)
-	pop_insn = emit_pop_insn (insn, regstack, reg1, where);
       if (get_hard_regnum (regstack, reg2) >= 0)
 	pop_insn = emit_pop_insn (insn, regstack, reg2, where);
+      if (get_hard_regnum (regstack, reg1) >= 0)
+	pop_insn = emit_pop_insn (insn, regstack, reg1, where);
       gcc_assert (pop_insn);
       return pop_insn;
     }
@@ -1374,10 +1374,14 @@ subst_stack_regs_pat (rtx insn, stack re
 			|= move_nan_for_stack_reg (insn, regstack, t);
 		    if (COMPLEX_MODE_P (GET_MODE (t)))
 		      {
-			t = FP_MODE_REG (REGNO (t) + 1, DFmode);
+			t = FP_MODE_REG (REGNO (t) + 1, SFmode);
 			if (get_hard_regnum (regstack, t) == -1)
-			  control_flow_insn_deleted
-			    |= move_nan_for_stack_reg (insn, regstack, t);
+			  {
+			    rtx pat2 = gen_rtx_CLOBBER (VOIDmode, t);
+			    rtx insn2 = emit_insn_before (pat2, insn);
+			    control_flow_insn_deleted
+			      |= move_nan_for_stack_reg (insn2, regstack, t);
+			  }
 		      }
 		  }
 	      }

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