This is the mail archive of the
libc-hacker@sourceware.cygnus.com
mailing list for the glibc project.
Re: i686 (PentiumPro) Optimizer Bug
- To: egcs-patches@cygnus.com
- Subject: Re: i686 (PentiumPro) Optimizer Bug
- From: hjl@lucon.org (H.J. Lu)
- Date: Fri, 18 Dec 1998 10:58:46 -0800 (PST)
- Cc: libc-hacker@cygnus.com (GNU C Library), linux-gcc@vger.rutgers.edu (linuxgcc)
>
> > Hi,
> >
> > I am looking into your bug report
> >
> > http://www.cygnus.com/ml/egcs-bugs/1998-Nov/0476.html
> >
> > I don't have a real fix yet. But I did find a bug in reg-stack.c which
> > would silently generate incorrect code. This patch is a sanity check
> > and it will cause compiler to abort on your testcase instead of
> > generating buggy asm code. I will work on a real fix when I find more
> > time.
> >
> > Thanks.
>
>
Hi,
This is a patch for egcs 1.1.1 which fixes the bug
http://www.cygnus.com/ml/egcs-bugs/1998-Nov/0476.html
The detailed analysises can be found at
http://www.cygnus.com/ml/egcs-bugs/1998-Dec/0475.html
http://www.cygnus.com/ml/egcs-bugs/1998-Dec/0481.html
If you are using floating point with Pentium Pro optimization, I
strongly encourage you to give this patch a try.
Thanks.
--
H.J. Lu (hjl@gnu.org)
---
Fri Dec 18 06:57:12 1998 H.J. Lu (hjl@gnu.org)
* reg-stack.c (subst_stack_regs_pat): Abort if the destination
of a FP conditional move is not on the FP register stack.
* config/i386/i386.md (movsfcc, movdfcc, movxfcc): Make sure the
target is among the sources.
(movsfcc+1, movsfcc+2, movsfcc+3, movsfcc+4, movsfcc+5, movdfcc+1,
movdfcc+2, movdfcc+3, movdfcc+4, movdfcc+5, movxfcc+1, movxfcc+2,
movxfcc+3, movxfcc+4, movxfcc+5): Disallow patterns where the target
is not among the sources.
* config/i386/i386.c (output_fp_conditional_move): Remove the
third alternative.
diff -x CVS -x c-gperf.h -upr /home/work/misc/gnu/import/egcs/gcc/config/i386/i386.c ./config/i386/i386.c
--- /home/work/misc/gnu/import/egcs/gcc/config/i386/i386.c Wed Jul 29 07:35:38 1998
+++ ./config/i386/i386.c Fri Dec 18 09:06:53 1998
@@ -5208,12 +5208,6 @@ output_fp_conditional_move (which_altern
output_asm_insn (AS2 (fcmov%f1,%3,%0), operands);
break;
- case 2:
- /* r <- cond ? r : arg */
- output_asm_insn (AS2 (fcmov%F1,%2,%0), operands);
- output_asm_insn (AS2 (fcmov%f1,%3,%0), operands);
- break;
-
default:
abort ();
}
diff -x CVS -x c-gperf.h -upr /home/work/misc/gnu/import/egcs/gcc/config/i386/i386.md ./config/i386/i386.md
--- /home/work/misc/gnu/import/egcs/gcc/config/i386/i386.md Thu Jul 30 08:11:16 1998
+++ ./config/i386/i386.md Fri Dec 18 08:47:30 1998
@@ -7383,6 +7383,15 @@ byte_xor_operation:
if (GET_MODE_CLASS (GET_MODE (i386_compare_op0)) != MODE_INT)
FAIL;
+ /* We have to make sure that the target is among the sources so
+ that it is on the register stack. */
+ if (!rtx_equal_p (operands[0], operands[2])
+ && !rtx_equal_p (operands[0], operands[3]))
+ {
+ emit_insn (gen_movsf (operands[0], operands[2]));
+ operands[2] = operands[0];
+ }
+
/* The floating point conditional move instructions don't directly
support conditions resulting from a signed integer comparison. */
@@ -7411,36 +7420,36 @@ byte_xor_operation:
}")
(define_insn ""
- [(set (match_operand:SF 0 "register_operand" "=f,f,f,f,f,f")
+ [(set (match_operand:SF 0 "register_operand" "=f,f,f,f")
(if_then_else:SF (match_operator 1 "comparison_operator"
- [(match_operand:QI 2 "nonimmediate_operand" "q,m,q,m,q,m")
- (match_operand:QI 3 "general_operand" "qmn,qn,qmn,qn,qmn,qn")])
- (match_operand:SF 4 "register_operand" "f,f,0,0,f,f")
- (match_operand:SF 5 "register_operand" "0,0,f,f,f,f")))]
+ [(match_operand:QI 2 "nonimmediate_operand" "q,m,q,m")
+ (match_operand:QI 3 "general_operand" "qmn,qn,qmn,qn")])
+ (match_operand:SF 4 "register_operand" "f,f,0,0")
+ (match_operand:SF 5 "register_operand" "0,0,f,f")))]
"TARGET_CMOVE
&& GET_CODE (operands[1]) != LT && GET_CODE (operands[1]) != LE
&& GET_CODE (operands[1]) != GE && GET_CODE (operands[1]) != GT"
"#")
(define_insn ""
- [(set (match_operand:SF 0 "register_operand" "=f,f,f,f,f,f")
+ [(set (match_operand:SF 0 "register_operand" "=f,f,f,f")
(if_then_else:SF (match_operator 1 "comparison_operator"
- [(match_operand 2 "nonimmediate_operand" "r,m,r,m,r,m")
- (match_operand 3 "general_operand" "rmi,ri,rmi,ri,rmi,ri")])
- (match_operand:SF 4 "register_operand" "f,f,0,0,f,f")
- (match_operand:SF 5 "register_operand" "0,0,f,f,f,f")))]
+ [(match_operand 2 "nonimmediate_operand" "r,m,r,m")
+ (match_operand 3 "general_operand" "rmi,ri,rmi,ri")])
+ (match_operand:SF 4 "register_operand" "f,f,0,0")
+ (match_operand:SF 5 "register_operand" "0,0,f,f")))]
"TARGET_CMOVE && GET_MODE_CLASS (GET_MODE (operands[2])) == MODE_INT
&& GET_CODE (operands[1]) != LT && GET_CODE (operands[1]) != LE
&& GET_CODE (operands[1]) != GE && GET_CODE (operands[1]) != GT"
"#")
(define_split
- [(set (match_operand:SF 0 "register_operand" "=f,f,f")
+ [(set (match_operand:SF 0 "register_operand" "=f,f")
(if_then_else:SF (match_operator 1 "comparison_operator"
[(match_operand 2 "nonimmediate_operand" "")
(const_int 0)])
- (match_operand:SF 3 "register_operand" "f,0,f")
- (match_operand:SF 4 "register_operand" "0,f,f")))]
+ (match_operand:SF 3 "register_operand" "f,0")
+ (match_operand:SF 4 "register_operand" "0,f")))]
"TARGET_CMOVE && reload_completed"
[(set (cc0)
(match_dup 2))
@@ -7450,12 +7459,12 @@ byte_xor_operation:
"")
(define_split
- [(set (match_operand:SF 0 "register_operand" "=f,f,f")
+ [(set (match_operand:SF 0 "register_operand" "=f,f")
(if_then_else:SF (match_operator 1 "comparison_operator"
[(match_operand 2 "nonimmediate_operand" "")
(match_operand 3 "general_operand" "")])
- (match_operand:SF 4 "register_operand" "f,0,f")
- (match_operand:SF 5 "register_operand" "0,f,f")))]
+ (match_operand:SF 4 "register_operand" "f,0")
+ (match_operand:SF 5 "register_operand" "0,f")))]
"TARGET_CMOVE && reload_completed"
[(set (cc0) (compare (match_dup 2) (match_dup 3)))
(set (match_dup 0)
@@ -7464,11 +7473,11 @@ byte_xor_operation:
"")
(define_insn ""
- [(set (match_operand:SF 0 "register_operand" "=f,f,f")
+ [(set (match_operand:SF 0 "register_operand" "=f,f")
(if_then_else:SF (match_operator 1 "comparison_operator"
[(cc0) (const_int 0)])
- (match_operand:SF 2 "register_operand" "f,0,f")
- (match_operand:SF 3 "register_operand" "0,f,f")))]
+ (match_operand:SF 2 "register_operand" "f,0")
+ (match_operand:SF 3 "register_operand" "0,f")))]
"TARGET_CMOVE && reload_completed"
"* return output_fp_conditional_move (which_alternative, operands);")
@@ -7485,6 +7494,15 @@ byte_xor_operation:
if (GET_MODE_CLASS (GET_MODE (i386_compare_op0)) != MODE_INT)
FAIL;
+ /* We have to make sure that the target is among the sources so
+ that it is on the register stack. */
+ if (!rtx_equal_p (operands[0], operands[2])
+ && !rtx_equal_p (operands[0], operands[3]))
+ {
+ emit_insn (gen_movdf (operands[0], operands[2]));
+ operands[2] = operands[0];
+ }
+
/* The floating point conditional move instructions don't directly
support conditions resulting from a signed integer comparison. */
@@ -7513,36 +7531,36 @@ byte_xor_operation:
}")
(define_insn ""
- [(set (match_operand:DF 0 "register_operand" "=f,f,f,f,f,f")
+ [(set (match_operand:DF 0 "register_operand" "=f,f,f,f")
(if_then_else:DF (match_operator 1 "comparison_operator"
- [(match_operand:QI 2 "nonimmediate_operand" "q,m,q,m,q,m")
- (match_operand:QI 3 "general_operand" "qmn,qn,qmn,qn,qmn,qn")])
- (match_operand:DF 4 "register_operand" "f,f,0,0,f,f")
- (match_operand:DF 5 "register_operand" "0,0,f,f,f,f")))]
+ [(match_operand:QI 2 "nonimmediate_operand" "q,m,q,m")
+ (match_operand:QI 3 "general_operand" "qmn,qn,qmn,qn")])
+ (match_operand:DF 4 "register_operand" "f,f,0,0")
+ (match_operand:DF 5 "register_operand" "0,0,f,f")))]
"TARGET_CMOVE
&& GET_CODE (operands[1]) != LT && GET_CODE (operands[1]) != LE
&& GET_CODE (operands[1]) != GE && GET_CODE (operands[1]) != GT"
"#")
(define_insn ""
- [(set (match_operand:DF 0 "register_operand" "=f,f,f,f,f,f")
+ [(set (match_operand:DF 0 "register_operand" "=f,f,f,f")
(if_then_else:DF (match_operator 1 "comparison_operator"
- [(match_operand 2 "nonimmediate_operand" "r,m,r,m,r,m")
- (match_operand 3 "general_operand" "rmi,ri,rmi,ri,rmi,ri")])
- (match_operand:DF 4 "register_operand" "f,f,0,0,f,f")
- (match_operand:DF 5 "register_operand" "0,0,f,f,f,f")))]
+ [(match_operand 2 "nonimmediate_operand" "r,m,r,m")
+ (match_operand 3 "general_operand" "rmi,ri,rmi,ri")])
+ (match_operand:DF 4 "register_operand" "f,f,0,0")
+ (match_operand:DF 5 "register_operand" "0,0,f,f")))]
"TARGET_CMOVE && GET_MODE_CLASS (GET_MODE (operands[2])) == MODE_INT
&& GET_CODE (operands[1]) != LT && GET_CODE (operands[1]) != LE
&& GET_CODE (operands[1]) != GE && GET_CODE (operands[1]) != GT"
"#")
(define_split
- [(set (match_operand:DF 0 "register_operand" "=f,f,f")
+ [(set (match_operand:DF 0 "register_operand" "=f,f")
(if_then_else:DF (match_operator 1 "comparison_operator"
[(match_operand 2 "nonimmediate_operand" "")
(const_int 0)])
- (match_operand:DF 3 "register_operand" "f,0,f")
- (match_operand:DF 4 "register_operand" "0,f,f")))]
+ (match_operand:DF 3 "register_operand" "f,0")
+ (match_operand:DF 4 "register_operand" "0,f")))]
"TARGET_CMOVE && reload_completed"
[(set (cc0)
(match_dup 2))
@@ -7552,12 +7570,12 @@ byte_xor_operation:
"")
(define_split
- [(set (match_operand:DF 0 "register_operand" "=f,f,f")
+ [(set (match_operand:DF 0 "register_operand" "=f,f")
(if_then_else:DF (match_operator 1 "comparison_operator"
[(match_operand 2 "nonimmediate_operand" "")
(match_operand 3 "general_operand" "")])
- (match_operand:DF 4 "register_operand" "f,0,f")
- (match_operand:DF 5 "register_operand" "0,f,f")))]
+ (match_operand:DF 4 "register_operand" "f,0")
+ (match_operand:DF 5 "register_operand" "0,f")))]
"TARGET_CMOVE && reload_completed"
[(set (cc0) (compare (match_dup 2) (match_dup 3)))
(set (match_dup 0)
@@ -7566,11 +7584,11 @@ byte_xor_operation:
"")
(define_insn ""
- [(set (match_operand:DF 0 "register_operand" "=f,f,f")
+ [(set (match_operand:DF 0 "register_operand" "=f,f")
(if_then_else:DF (match_operator 1 "comparison_operator"
[(cc0) (const_int 0)])
- (match_operand:DF 2 "register_operand" "f,0,f")
- (match_operand:DF 3 "register_operand" "0,f,f")))]
+ (match_operand:DF 2 "register_operand" "f,0")
+ (match_operand:DF 3 "register_operand" "0,f")))]
"TARGET_CMOVE && reload_completed"
"* return output_fp_conditional_move (which_alternative, operands);")
@@ -7587,6 +7605,15 @@ byte_xor_operation:
if (GET_MODE_CLASS (GET_MODE (i386_compare_op0)) != MODE_INT)
FAIL;
+ /* We have to make sure that the target is among the sources so
+ that it is on the register stack. */
+ if (!rtx_equal_p (operands[0], operands[2])
+ && !rtx_equal_p (operands[0], operands[3]))
+ {
+ emit_insn (gen_movxf (operands[0], operands[2]));
+ operands[2] = operands[0];
+ }
+
/* The floating point conditional move instructions don't directly
support conditions resulting from a signed integer comparison. */
@@ -7615,36 +7642,36 @@ byte_xor_operation:
}")
(define_insn ""
- [(set (match_operand:XF 0 "register_operand" "=f,f,f,f,f,f")
+ [(set (match_operand:XF 0 "register_operand" "=f,f,f,f")
(if_then_else:XF (match_operator 1 "comparison_operator"
- [(match_operand:QI 2 "nonimmediate_operand" "q,m,q,m,q,m")
- (match_operand:QI 3 "general_operand" "qmn,qn,qmn,qn,qmn,qn")])
- (match_operand:XF 4 "register_operand" "f,f,0,0,f,f")
- (match_operand:XF 5 "register_operand" "0,0,f,f,f,f")))]
+ [(match_operand:QI 2 "nonimmediate_operand" "q,m,q,m")
+ (match_operand:QI 3 "general_operand" "qmn,qn,qmn,qn")])
+ (match_operand:XF 4 "register_operand" "f,f,0,0")
+ (match_operand:XF 5 "register_operand" "0,0,f,f")))]
"TARGET_CMOVE
&& GET_CODE (operands[1]) != LT && GET_CODE (operands[1]) != LE
&& GET_CODE (operands[1]) != GE && GET_CODE (operands[1]) != GT"
"#")
(define_insn ""
- [(set (match_operand:XF 0 "register_operand" "=f,f,f,f,f,f")
+ [(set (match_operand:XF 0 "register_operand" "=f,f,f,f")
(if_then_else:XF (match_operator 1 "comparison_operator"
- [(match_operand 2 "nonimmediate_operand" "r,m,r,m,r,m")
- (match_operand 3 "general_operand" "rmi,ri,rmi,ri,rmi,ri")])
- (match_operand:XF 4 "register_operand" "f,f,0,0,f,f")
- (match_operand:XF 5 "register_operand" "0,0,f,f,f,f")))]
+ [(match_operand 2 "nonimmediate_operand" "r,m,r,m")
+ (match_operand 3 "general_operand" "rmi,ri,rmi,ri")])
+ (match_operand:XF 4 "register_operand" "f,f,0,0")
+ (match_operand:XF 5 "register_operand" "0,0,f,f")))]
"TARGET_CMOVE && GET_MODE_CLASS (GET_MODE (operands[2])) == MODE_INT
&& GET_CODE (operands[1]) != LT && GET_CODE (operands[1]) != LE
&& GET_CODE (operands[1]) != GE && GET_CODE (operands[1]) != GT"
"#")
(define_split
- [(set (match_operand:XF 0 "register_operand" "=f,f,f")
+ [(set (match_operand:XF 0 "register_operand" "=f,f")
(if_then_else:XF (match_operator 1 "comparison_operator"
[(match_operand 2 "nonimmediate_operand" "")
(const_int 0)])
- (match_operand:XF 3 "register_operand" "f,0,f")
- (match_operand:XF 4 "register_operand" "0,f,f")))]
+ (match_operand:XF 3 "register_operand" "f,0")
+ (match_operand:XF 4 "register_operand" "0,f")))]
"TARGET_CMOVE && reload_completed"
[(set (cc0)
(match_dup 2))
@@ -7654,12 +7681,12 @@ byte_xor_operation:
"")
(define_split
- [(set (match_operand:XF 0 "register_operand" "=f,f,f")
+ [(set (match_operand:XF 0 "register_operand" "=f,f")
(if_then_else:XF (match_operator 1 "comparison_operator"
[(match_operand 2 "nonimmediate_operand" "")
(match_operand 3 "general_operand" "")])
- (match_operand:XF 4 "register_operand" "f,0,f")
- (match_operand:XF 5 "register_operand" "0,f,f")))]
+ (match_operand:XF 4 "register_operand" "f,0")
+ (match_operand:XF 5 "register_operand" "0,f")))]
"TARGET_CMOVE && reload_completed"
[(set (cc0) (compare (match_dup 2) (match_dup 3)))
(set (match_dup 0)
@@ -7668,11 +7695,11 @@ byte_xor_operation:
"")
(define_insn ""
- [(set (match_operand:XF 0 "register_operand" "=f,f,f")
+ [(set (match_operand:XF 0 "register_operand" "=f,f")
(if_then_else:XF (match_operator 1 "comparison_operator"
[(cc0) (const_int 0)])
- (match_operand:XF 2 "register_operand" "f,0,f")
- (match_operand:XF 3 "register_operand" "0,f,f")))]
+ (match_operand:XF 2 "register_operand" "f,0")
+ (match_operand:XF 3 "register_operand" "0,f")))]
"TARGET_CMOVE && reload_completed"
"* return output_fp_conditional_move (which_alternative, operands);")
diff -x CVS -x c-gperf.h -upr /home/work/misc/gnu/import/egcs/gcc/reg-stack.c ./reg-stack.c
--- /home/work/misc/gnu/import/egcs/gcc/reg-stack.c Thu Oct 8 12:10:13 1998
+++ ./reg-stack.c Wed Dec 16 14:45:00 1998
@@ -2349,6 +2349,10 @@ subst_stack_regs_pat (insn, regstack, pa
break;
case IF_THEN_ELSE:
+ /* dest has to be on stack. */
+ if (get_hard_regnum (regstack, *dest) < FIRST_STACK_REG)
+ abort ();
+
/* This insn requires the top of stack to be the destination. */
/* If the comparison operator is an FP comparison operator,
@@ -2402,9 +2406,7 @@ subst_stack_regs_pat (insn, regstack, pa
}
}
- /* Make dest the top of stack. Add dest to regstack if not present. */
- if (get_hard_regnum (regstack, *dest) < FIRST_STACK_REG)
- regstack->reg[++regstack->top] = REGNO (*dest);
+ /* Make dest the top of stack. */
SET_HARD_REG_BIT (regstack->reg_set, REGNO (*dest));
replace_reg (dest, FIRST_STACK_REG);