This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [RFC] PR/27390, latent complex-mode bug in regstack
- From: Paolo Bonzini <paolo dot bonzini at lu dot unisi dot ch>
- To: Paolo Bonzini <paolo dot bonzini at lu dot unisi dot ch>, GCC Patches <gcc-patches at gcc dot gnu dot org>
- Date: Mon, 05 Jun 2006 19:44:46 +0200
- Subject: Re: [RFC] PR/27390, latent complex-mode bug in regstack
- References: <44846CA9.2060902@lu.unisi.ch>
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);
+ }
}
}
}