This is the mail archive of the
gcc@gcc.gnu.org
mailing list for the GCC project.
Re: i386 FP comparsion bug (new patch)
- To: law at cygnus dot com
- Subject: Re: i386 FP comparsion bug (new patch)
- From: Jan Hubicka <hubicka at atrey dot karlin dot mff dot cuni dot cz>
- Date: Sat, 31 Oct 1998 21:12:52 +0100
- Cc: egcs at cygnus dot com
- References: <19981026090505.29312@atrey.karlin.mff.cuni.cz> <2834.909726230@hurl.cygnus.com>
Hi
So here is my second approach to fix the fp comparsion problem. This
patch modifies reg-stack to not use delay slot trick to pop second
operand and put REG_DEAD note instead. Operand is popped by
output_float_compare in i386.c
This patch is tested only with my quite heavily modified i386
machine description, but I don't see any purpose why it should not work
in egcs. Hope that this patch covers all cases and don't break anything.
I will probably do some more testing at monday. (running testsuite
and so on). For now I've succesfully bootstraped and compiled XaoS
(it didn't worked with -mno-ieee-fp, now it seems to work)
BTW I've claimed that this bug occours with normal compilation too,
but it is probably not true, because reg_stack probably never emits reversed
comparsion (I will try to modify it to do so soon). So this bug happends
only in -mno-ieee-fp mode.
Honza
Sat Oct 31 05:08:34 CET 1998 Jan Hubicka (hubicka@freesoft.cz)
* reg-stack.c: Do not emit pop insns after cc0 setter.
(emit_pop_insn): Do not emit insn in case WHEN is NULL.
(compare_for_stack_reg): Update REG_DEAD note and
do not emit push insn.
* i386.c:
(output_float_compare): Handle new REG_DEAD notes.
*** gcc/reg-stack.old Sat Oct 31 03:27:03 1998
--- gcc/reg-stack.c Sat Oct 31 05:06:07 1998
*************** delete_insn_for_stacker (insn)
*** 1700,1707 ****
/* Emit an insn to pop virtual register REG before or after INSN.
REGSTACK is the stack state after INSN and is updated to reflect this
! pop. WHEN is either emit_insn_before or emit_insn_after. A pop insn
! is represented as a SET whose destination is the register to be popped
and source is the top of stack. A death note for the top of stack
cases the movdf pattern to pop. */
--- 1700,1710 ----
/* Emit an insn to pop virtual register REG before or after INSN.
REGSTACK is the stack state after INSN and is updated to reflect this
! pop. WHEN is either emit_insn_before, emit_insn_after or NULL.
! in case WHEN is NULL we don't really emit the insn, just modify stack
! information. Caller is expected to emit insn himself.
!
! A pop insn is represented as a SET whose destination is the register to be popped
and source is the top of stack. A death note for the top of stack
cases the movdf pattern to pop. */
*************** emit_pop_insn (insn, regstack, reg, when
*** 1720,1735 ****
if (hard_regno < FIRST_STACK_REG)
abort ();
! pop_rtx = gen_rtx_SET (VOIDmode, FP_MODE_REG (hard_regno, DFmode),
! FP_MODE_REG (FIRST_STACK_REG, DFmode));
!
! pop_insn = (*when) (pop_rtx, insn);
! /* ??? This used to be VOIDmode, but that seems wrong. */
! PUT_MODE (pop_insn, QImode);
!
! REG_NOTES (pop_insn) = gen_rtx_EXPR_LIST (REG_DEAD,
! FP_MODE_REG (FIRST_STACK_REG, DFmode),
! REG_NOTES (pop_insn));
regstack->reg[regstack->top - (hard_regno - FIRST_STACK_REG)]
= regstack->reg[regstack->top];
--- 1723,1741 ----
if (hard_regno < FIRST_STACK_REG)
abort ();
! if (when)
! {
! pop_rtx = gen_rtx_SET (VOIDmode, FP_MODE_REG (hard_regno, DFmode),
! FP_MODE_REG (FIRST_STACK_REG, DFmode));
!
! pop_insn = (*when) (pop_rtx, insn);
! /* ??? This used to be VOIDmode, but that seems wrong. */
! PUT_MODE (pop_insn, QImode);
!
! REG_NOTES (pop_insn) = gen_rtx_EXPR_LIST (REG_DEAD,
! FP_MODE_REG (FIRST_STACK_REG, DFmode),
! REG_NOTES (pop_insn));
! }
regstack->reg[regstack->top - (hard_regno - FIRST_STACK_REG)]
= regstack->reg[regstack->top];
*************** swap_rtx_condition (pat)
*** 1992,2001 ****
/* Handle a comparison. Special care needs to be taken to avoid
causing comparisons that a 387 cannot do correctly, such as EQ.
! Also, a pop insn may need to be emitted. The 387 does have an
`fcompp' insn that can pop two regs, but it is sometimes too expensive
to do this - a `fcomp' followed by a `fstpl %st(0)' may be easier to
! set up. */
static void
compare_for_stack_reg (insn, regstack, pat)
--- 1998,2015 ----
/* Handle a comparison. Special care needs to be taken to avoid
causing comparisons that a 387 cannot do correctly, such as EQ.
! Also, a fstp instruction may need to be emitted. The 387 does have an
`fcompp' insn that can pop two regs, but it is sometimes too expensive
to do this - a `fcomp' followed by a `fstpl %st(0)' may be easier to
! set up.
!
! We can not handle this by emiting fpop instruction after compare, because
! it appears between cc0 setter and user. So we emit only
! REG_DEAD note and handle it as a special case in machine description.
!
! This code used trick with delay_slot filling to emit pop insn after
! comparsion but it didn't worked because it caused confusion with cc_status
! in final pass. */
static void
compare_for_stack_reg (insn, regstack, pat)
*************** compare_for_stack_reg (insn, regstack, p
*** 2007,2012 ****
--- 2021,2027 ----
rtx src1_note, src2_note;
rtx cc0_user;
int have_cmove;
+ int hard_regno;
src1 = get_true_reg (&XEXP (SET_SRC (pat), 0));
src2 = get_true_reg (&XEXP (SET_SRC (pat), 1));
*************** compare_for_stack_reg (insn, regstack, p
*** 2073,2079 ****
replace_reg (src1, FIRST_STACK_REG);
if (STACK_REG_P (*src2))
! replace_reg (src2, get_hard_regnum (regstack, *src2));
if (src1_note)
{
--- 2088,2097 ----
replace_reg (src1, FIRST_STACK_REG);
if (STACK_REG_P (*src2))
! {
! hard_regno = get_hard_regnum (regstack, *src2);
! replace_reg (src2, hard_regno);
! }
if (src1_note)
{
*************** compare_for_stack_reg (insn, regstack, p
*** 2102,2117 ****
}
else
{
! /* The 386 can only represent death of the first operand in
! the case handled above. In all other cases, emit a separate
! pop and remove the death note from here. */
!
! link_cc0_insns (insn);
!
! remove_regno_note (insn, REG_DEAD, REGNO (XEXP (src2_note, 0)));
! emit_pop_insn (insn, regstack, XEXP (src2_note, 0),
! emit_insn_after);
}
}
}
--- 2120,2130 ----
}
else
{
! /* Pop of second operand is handled using special REG_DEAD note
! because we can't emit pop insn after cc0 setter */
! emit_pop_insn (insn, regstack, XEXP (src2_note, 0), NULL);
! replace_reg (&XEXP (src2_note, 0), hard_regno);
}
}
}
*** gcc/config/i386/i386.c.orig Tue Oct 27 18:09:27 1998
--- gcc/config/i386/i386.c Sat Oct 31 05:06:22 1998
*************** output_float_compare (insn, operands)
*** 3976,3981 ****
--- 4025,4032 ----
rtx body = XVECEXP (PATTERN (insn), 0, 0);
int unordered_compare = GET_MODE (SET_SRC (body)) == CCFPEQmode;
rtx tmp;
+ int cc0_set = 1;
+ int i;
if (0 && TARGET_CMOVE && STACK_REG_P (operands[1]))
{
*************** output_float_compare (insn, operands)
*** 3999,4005 ****
if (STACK_REG_P (operands[1])
&& stack_top_dies
&& find_regno_note (insn, REG_DEAD, REGNO (operands[1]))
! && REGNO (operands[1]) != FIRST_STACK_REG)
{
/* If both the top of the 387 stack dies, and the other operand
is also a stack register that dies, then this must be a
--- 4051,4057 ----
if (STACK_REG_P (operands[1])
&& stack_top_dies
&& find_regno_note (insn, REG_DEAD, REGNO (operands[1]))
! && REGNO (operands[1]) == FIRST_STACK_REG + 1)
{
/* If both the top of the 387 stack dies, and the other operand
is also a stack register that dies, then this must be a
*************** output_float_compare (insn, operands)
*** 4011,4017 ****
{
output_asm_insn (AS2 (fucomip,%y1,%0), operands);
output_asm_insn (AS1 (fstp, %y0), operands);
! return "";
}
else
output_asm_insn ("fucompp", operands);
--- 4063,4069 ----
{
output_asm_insn (AS2 (fucomip,%y1,%0), operands);
output_asm_insn (AS1 (fstp, %y0), operands);
! cc0_set = 0;
}
else
output_asm_insn ("fucompp", operands);
*************** output_float_compare (insn, operands)
*** 4022,4028 ****
{
output_asm_insn (AS2 (fcomip, %y1,%0), operands);
output_asm_insn (AS1 (fstp, %y0), operands);
! return "";
}
else
output_asm_insn ("fcompp", operands);
--- 4074,4080 ----
{
output_asm_insn (AS2 (fcomip, %y1,%0), operands);
output_asm_insn (AS1 (fstp, %y0), operands);
! cc0_set = 0;
}
else
output_asm_insn ("fcompp", operands);
*************** output_float_compare (insn, operands)
*** 4052,4066 ****
else if (cc_status.flags & CC_FCOMI)
{
output_asm_insn (strcat (buf, AS2 (%z1,%y1,%0)), operands);
! return "";
}
else
output_asm_insn (strcat (buf, AS1 (%z1,%y1)), operands);
}
/* Now retrieve the condition code. */
- return output_fp_cc0_set (insn);
}
/* Output opcodes to transfer the results of FP compare or test INSN
--- 4104,4140 ----
else if (cc_status.flags & CC_FCOMI)
{
output_asm_insn (strcat (buf, AS2 (%z1,%y1,%0)), operands);
! cc0_set = 0;
}
else
output_asm_insn (strcat (buf, AS1 (%z1,%y1)), operands);
}
/* Now retrieve the condition code. */
+ if (cc0_set)
+ {
+ char *r = output_fp_cc0_set (insn);
+ if (r[0]) output_asm_insn (r, operands);
+ }
+
+
+ /* We emit fstp instruction after integer comparsions to improve scheduling */
+ for (i = 0; i < 2 ; i++)
+ {
+ if (STACK_REG_P (operands[i])
+ && find_regno_note (insn, REG_DEAD, REGNO (operands[i]))
+ && REGNO (operands[i]) != FIRST_STACK_REG
+ && (!stack_top_dies || REGNO (operands[i]) != FIRST_STACK_REG + 1))
+ {
+ rtx xexp[i];
+ xexp[0] = gen_rtx_REG (DFmode, REGNO (operands[i]) - (stack_top_dies != 0));
+ output_asm_insn (AS1 (fstp, %y0), xexp);
+ }
+ }
+
+ return "";
+
}
/* Output opcodes to transfer the results of FP compare or test INSN