This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
[PATCH] Fix PR rtl-optimization/19296
- From: Eric Botcazou <ebotcazou at libertysurf dot fr>
- To: gcc-patches at gcc dot gnu dot org
- Date: Sun, 9 Jan 2005 00:16:43 +0100
- Subject: [PATCH] Fix PR rtl-optimization/19296
Hi,
This is a wrong code generation at -O on i686 for:
void f(unsigned short ad)
{
if (ad >= 0x4000 && ad < 0xc000)
abort();
}
int main(void)
{
f(0xff00);
return 0;
}
which gets turned by fold into:
void f1(unsigned short ad)
{
if ((short) (ad - 0x4000) >= 0)
abort ();
}
a regression from GCC 2.95.3 present in all 3.x compilers as well as in
4.0.0pre.
The problem originates in the combiner: when fed with
(insn 4 3 6 0 (set (reg/v:SI 58 [ ad ])
(zero_extend:SI (subreg:HI (reg:SI 59 [ ad ]) 0))) 75
{*zero_extendhisi2_movzwl} (insn_list 3 (nil))
(expr_list:REG_DEAD (reg:SI 59 [ ad ])
(nil)))
[...]
(insn 12 6 13 0 (parallel [
(set (reg:SI 61)
(plus:SI (reg/v:SI 58 [ ad ])
(const_int -16384 [0xffffc000])))
(clobber (reg:CC 17 flags))
]) 139 {*addsi_1} (insn_list 4 (nil))
(expr_list:REG_DEAD (reg/v:SI 58 [ ad ])
(expr_list:REG_UNUSED (reg:CC 17 flags)
(nil))))
(insn 13 12 14 0 (set (reg:CCGOC 17 flags)
(compare:CCGOC (subreg:HI (reg:SI 61) 0)
(const_int 0 [0x0]))) 3 {*cmphi_ccno_1} (insn_list 12 (nil))
(expr_list:REG_DEAD (reg:SI 61)
(nil)))
it produces:
(insn 4 3 6 0 (set (reg/v:SI 58 [ ad ])
(zero_extend:SI (mem/f:HI (reg/f:SI 16 argp) [0 ad+0 S2 A32]))) 75
{*zero_extendhisi2_movzwl} (nil)
(nil))
[...]
(note 12 6 13 0 NOTE_INSN_DELETED)
(insn 13 12 14 0 (parallel [
(set (reg:CCGOC 17 flags)
(compare:CCGOC (plus:SI (reg/v:SI 58 [ ad ])
(const_int -16384 [0xffffc000]))
(const_int 0 [0x0])))
(clobber (scratch:SI))
]) 143 {addsi_5} (insn_list 4 (nil))
(expr_list:REG_UNUSED (scratch:SI)
(expr_list:REG_DEAD (reg/v:SI 58 [ ad ])
(nil))))
that is, the compare has been wrongly promoted from HImode to SImode.
The code responsible for doing the promotion lies in simplify_comparison:
case SUBREG:
/* Check for the case where we are comparing A - C1 with C2,
both constants are smaller than 1/2 the maximum positive
value in MODE, and the comparison is equality or unsigned.
In that case, if A is either zero-extended to MODE or has
sufficient sign bits so that the high-order bit in MODE
is a copy of the sign in the inner mode, we can prove that it is
safe to do the operation in the wider mode. This simplifies
many range checks. */
if (mode_width <= HOST_BITS_PER_WIDE_INT
&& subreg_lowpart_p (op0)
&& GET_CODE (SUBREG_REG (op0)) == PLUS
&& GET_CODE (XEXP (SUBREG_REG (op0), 1)) == CONST_INT
&& INTVAL (XEXP (SUBREG_REG (op0), 1)) < 0
&& (-INTVAL (XEXP (SUBREG_REG (op0), 1))
< (HOST_WIDE_INT) (GET_MODE_MASK (mode) / 2))
&& (unsigned HOST_WIDE_INT) const_op < GET_MODE_MASK (mode) / 2
&& (0 == (nonzero_bits (XEXP (SUBREG_REG (op0), 0),
GET_MODE (SUBREG_REG (op0)))
& ~GET_MODE_MASK (mode))
|| (num_sign_bit_copies (XEXP (SUBREG_REG (op0), 0),
GET_MODE (SUBREG_REG (op0)))
> (unsigned int)
(GET_MODE_BITSIZE (GET_MODE (SUBREG_REG (op0)))
- GET_MODE_BITSIZE (mode)))))
{
op0 = SUBREG_REG (op0);
continue;
}
and turns:
(set (reg:CCGOC 17 flags)
(compare:CCGOC (subreg:HI (plus:SI (reg/v:SI 58 [ ad ])
(const_int -16384 [0xffffc000])) 0)
(const_int 0 [0x0])))
into
(set (reg:CCGOC 17 flags)
(compare:CCGOC (plus:SI (reg/v:SI 58 [ ad ])
(const_int -16384 [0xffffc000]))
(const_int 0 [0x0])))
Note that the comment says that the transformation is only valid for equality
and unsigned comparisons, yet the code doesn't check it. For our testcase
the comparison is indeed signed (LT). Also note that the code is present
essentially unmodified in GCC 2.8.1.
I think that there are 2 problems with the transformation:
- signed vs unsigned/equality,
- the sign of the constant C2 matters, probably since the constants are
sign-extended for their mode.
Bootstrapped/regtested on amd64-mandrake-linux-gnu.
2005-01-08 Eric Botcazou <ebotcazou@libertysurf.fr>
PR rtl-optimization/19296
* combine.c (simplify_comparison): Rewrite the condition under
which a non-paradoxical SUBREG of a PLUS can be lifted when
compared against a constant.
2005-01-08 Eric Botcazou <ebotcazou@libertysurf.fr>
* gcc.dg/short-compare-1.c: New test.
* gcc.dg/short-compare-2.c: Likewise.
--
Eric Botcazou
/* PR rtl-optimization/19296 */
/* Origin: Falk Hueffner <falk@debian.org> */
/* { dg-do run } */
/* { dg-options "-O" } */
/* { dg-options "-O -mtune=i686" { target i?86-*-* } } */
/* { dg-options "-O -m32 -mtune=i686" { target x86_64-*-* } } */
extern void abort(void);
void f(unsigned short ad)
{
if (ad >= 0x4000 && ad < 0xc000)
abort();
}
int main(void)
{
f(0xff00);
return 0;
}
Index: combine.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/combine.c,v
retrieving revision 1.400.4.11
diff -c -p -r1.400.4.11 combine.c
*** combine.c 12 Oct 2004 23:35:29 -0000 1.400.4.11
--- combine.c 8 Jan 2005 23:02:42 -0000
*************** simplify_comparison (enum rtx_code code,
*** 10688,10721 ****
break;
case SUBREG:
! /* Check for the case where we are comparing A - C1 with C2,
! both constants are smaller than 1/2 the maximum positive
! value in MODE, and the comparison is equality or unsigned.
! In that case, if A is either zero-extended to MODE or has
! sufficient sign bits so that the high-order bit in MODE
! is a copy of the sign in the inner mode, we can prove that it is
! safe to do the operation in the wider mode. This simplifies
! many range checks. */
if (mode_width <= HOST_BITS_PER_WIDE_INT
&& subreg_lowpart_p (op0)
&& GET_CODE (SUBREG_REG (op0)) == PLUS
! && GET_CODE (XEXP (SUBREG_REG (op0), 1)) == CONST_INT
! && INTVAL (XEXP (SUBREG_REG (op0), 1)) < 0
! && (-INTVAL (XEXP (SUBREG_REG (op0), 1))
! < (HOST_WIDE_INT) (GET_MODE_MASK (mode) / 2))
! && (unsigned HOST_WIDE_INT) const_op < GET_MODE_MASK (mode) / 2
! && (0 == (nonzero_bits (XEXP (SUBREG_REG (op0), 0),
! GET_MODE (SUBREG_REG (op0)))
! & ~GET_MODE_MASK (mode))
! || (num_sign_bit_copies (XEXP (SUBREG_REG (op0), 0),
! GET_MODE (SUBREG_REG (op0)))
! > (unsigned int)
! (GET_MODE_BITSIZE (GET_MODE (SUBREG_REG (op0)))
! - GET_MODE_BITSIZE (mode)))))
{
! op0 = SUBREG_REG (op0);
! continue;
}
/* If the inner mode is narrower and we are extracting the low part,
--- 10688,10748 ----
break;
case SUBREG:
! /* Check for the case where we are comparing A - C1 with C2, that is
!
! (subreg:MODE (plus (A) (-C1))) op (C2)
!
! with C1 a constant, and try to lift the SUBREG, i.e. to do the
! comparison in the wider mode. One of the following two conditions
! must be true in order for this to be valid:
!
! 1. The mode extension results in the same bit pattern being added
! on both sides and the comparison is equality or unsigned. As
! C2 has been truncated to fit in MODE, the pattern can only be
! all 0s or all 1s.
!
! 2. The mode extension results in the sign bit being copied on
! each side.
!
! The difficulty here is that we have predicates for A but not for
! (A - C1) so we need to check that C1 is within proper bounds so
! as to perturbate A as little as possible. */
if (mode_width <= HOST_BITS_PER_WIDE_INT
&& subreg_lowpart_p (op0)
+ && GET_MODE_BITSIZE (GET_MODE (SUBREG_REG (op0))) > mode_width
&& GET_CODE (SUBREG_REG (op0)) == PLUS
! && GET_CODE (XEXP (SUBREG_REG (op0), 1)) == CONST_INT)
{
! enum machine_mode inner_mode = GET_MODE (SUBREG_REG (op0));
! rtx a = XEXP (SUBREG_REG (op0), 0);
! HOST_WIDE_INT c1 = -INTVAL (XEXP (SUBREG_REG (op0), 1));
!
! if ((c1 > 0
! && (unsigned HOST_WIDE_INT) c1
! < (unsigned HOST_WIDE_INT) 1 << (mode_width - 1)
! && (equality_comparison_p || unsigned_comparison_p)
! /* (A - C1) zero-extends if it is positive and sign-extends
! if it is negative, C2 both zero- and sign-extends. */
! && ((0 == (nonzero_bits (a, inner_mode)
! & ~GET_MODE_MASK (mode))
! && const_op >= 0)
! /* (A - C1) sign-extends if it is positive and 1-extends
! if it is negative, C2 both sign- and 1-extends. */
! || (num_sign_bit_copies (a, inner_mode)
! > (unsigned int) (GET_MODE_BITSIZE (inner_mode)
! - mode_width)
! && const_op < 0)))
! || ((unsigned HOST_WIDE_INT) c1
! < (unsigned HOST_WIDE_INT) 1 << (mode_width - 2)
! /* (A - C1) always sign-extends, like C2. */
! && num_sign_bit_copies (a, inner_mode)
! > (unsigned int) (GET_MODE_BITSIZE (inner_mode)
! - mode_width - 1)))
! {
! op0 = SUBREG_REG (op0);
! continue;
! }
}
/* If the inner mode is narrower and we are extracting the low part,
/* PR rtl-optimization/19296 */
/* Origin: Falk Hueffner <falk@debian.org> */
/* Testcase by Andrew Pinski <pinskia@gcc.gnu.org> */
/* { dg-do run } */
/* { dg-options "-O" } */
/* { dg-options "-O -mtune=i686" { target i?86-*-* } } */
/* { dg-options "-O -m32 -mtune=i686" { target x86_64-*-* } } */
extern void abort ();
void f(unsigned short ad)
{
if ((short) (ad - 0x4000) >= 0)
abort ();
}
int main (void)
{
f(0xc000);
return 0;
}