* Re: patch to stop GCC from shifting by a negative number
@ 2002-08-13 12:46 Steve Ellcey
2002-08-13 12:56 ` Geoff Keating
0 siblings, 1 reply; 7+ messages in thread
From: Steve Ellcey @ 2002-08-13 12:46 UTC (permalink / raw)
To: geoffk; +Cc: gcc-patches
> + /* You want to truncate to a _what_? */
> + if (GET_MODE_CLASS (mode) != MODE_INT
> + && GET_MODE_CLASS (mode) != MODE_PARTIAL_INT)
> + abort ();
> +
I ran the test suite with your patch and hit this abort a couple of
times on the PA64 HP-UX platform with tests
gcc.c-torture/execute/20020227-1.c and gcc.c-torture/compile/simd-1.c
using -O2 optmization.
In 20020227-1.c I enter simplify_binary_operation
with
code=LSHIFTRT
mode=SCmode
op0=(const_int -1 [0xffffffffffffffff])
op1=(const_int 32 [0x20])
In simd-1.c I enter simplify_binary_operation
with
code=ASHIFT
mode=V2SImode
op0=(const_int 4294967295 [0xffffffff])
op1=(const_int 32 [0x20])
and in both cases I call trunc_int_for_mode right at the end of
simplify_binary_operation with a mode of either SCmode or V2SImode which
then causes the abort to be hit.
Any ideas on what a good fix for these cases would be?
Steve Ellcey
sje@cup.hp.com
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: patch to stop GCC from shifting by a negative number
2002-08-13 12:46 patch to stop GCC from shifting by a negative number Steve Ellcey
@ 2002-08-13 12:56 ` Geoff Keating
0 siblings, 0 replies; 7+ messages in thread
From: Geoff Keating @ 2002-08-13 12:56 UTC (permalink / raw)
To: Steve Ellcey; +Cc: gcc-patches
Steve Ellcey <sje@cup.hp.com> writes:
> > + /* You want to truncate to a _what_? */
> > + if (GET_MODE_CLASS (mode) != MODE_INT
> > + && GET_MODE_CLASS (mode) != MODE_PARTIAL_INT)
> > + abort ();
> > +
>
> I ran the test suite with your patch and hit this abort a couple of
> times on the PA64 HP-UX platform with tests
> gcc.c-torture/execute/20020227-1.c and gcc.c-torture/compile/simd-1.c
> using -O2 optmization.
>
> In 20020227-1.c I enter simplify_binary_operation
> with
>
> code=LSHIFTRT
> mode=SCmode
> op0=(const_int -1 [0xffffffffffffffff])
> op1=(const_int 32 [0x20])
>
> In simd-1.c I enter simplify_binary_operation
> with
>
> code=ASHIFT
> mode=V2SImode
> op0=(const_int 4294967295 [0xffffffff])
> op1=(const_int 32 [0x20])
>
> and in both cases I call trunc_int_for_mode right at the end of
> simplify_binary_operation with a mode of either SCmode or V2SImode which
> then causes the abort to be hit.
>
> Any ideas on what a good fix for these cases would be?
I think the bug must be before this (that is,
simplify_binary_operation shouldn't be called this way) because a
right-shift of a complex number is nonsense. From context, I think it
should be called with DImode. I bet this is coming from
simplify_subreg or similar and someone used the wrong mode variable.
--
- Geoffrey Keating <geoffk@geoffk.org> <geoffk@redhat.com>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: patch to stop GCC from shifting by a negative number
@ 2002-08-16 14:09 Steve Ellcey
2002-08-17 10:49 ` Richard Henderson
0 siblings, 1 reply; 7+ messages in thread
From: Steve Ellcey @ 2002-08-16 14:09 UTC (permalink / raw)
To: geoffk, rth; +Cc: gcc-patches
Here is a new patch to stop GCC from shifting by a negative number in
trunc_int_for_mode. It is basically Geoff Keating's patch
(http://gcc.gnu.org/ml/gcc-patches/2002-08/msg00853.html) that ensures
we don't call trunc_int_for_mode with VOIDmode but with a couple of
extra changes in combine.c to stop us from calling it with Vector or
Complex modes either.
With Geoff's original patch I was failing
gcc.c-torture/execute/20020227-1.c and gcc.c-torture/compile/simd-1.c,
with them it passes the test suite with no regressions.
Here is a patch that includes both my changes and Geoff's.
Steve Ellcey
sje@cup.hp.com
2002-08-12 Geoffrey Keating <geoffk@redhat.com>
Steve Ellcey <sje@cup.hp.com>
* explow.c (trunc_int_for_mode): Abort when the mode is not
an integer mode.
* combine.c (expand_compound_operation): Don't expand Vector
or Complex modes into shifts.
(expand_field_assignment): Don't do bitwise arithmatic and
shifts on Vector or Complex modes.
(simplify_comparison): Don't call trunc_int_for_mode
for VOIDmode.
* recog.c (general_operand): Likewise.
(immediate_operand): Likewise.
(nonmemory_operand): Likewise.
*** gcc.orig/gcc/explow.c Fri Aug 16 12:12:13 2002
--- gcc/gcc/explow.c Fri Aug 16 12:12:55 2002
*************** trunc_int_for_mode (c, mode)
*** 49,54 ****
--- 49,59 ----
{
int width = GET_MODE_BITSIZE (mode);
+ /* You want to truncate to a _what_? */
+ if (GET_MODE_CLASS (mode) != MODE_INT
+ && GET_MODE_CLASS (mode) != MODE_PARTIAL_INT)
+ abort ();
+
/* Canonicalize BImode to 0 and STORE_FLAG_VALUE. */
if (mode == BImode)
return c & 1 ? STORE_FLAG_VALUE : 0;
*** gcc.orig/gcc/combine.c Fri Aug 16 12:12:17 2002
--- gcc/gcc/combine.c Fri Aug 16 12:20:23 2002
*************** expand_compound_operation (x)
*** 5634,5639 ****
--- 5634,5646 ----
if (GET_MODE_SIZE (GET_MODE (XEXP (x, 0))) > UNITS_PER_WORD)
return x;
+ /* Reject MODEs that aren't MODE_INT or MODE_PARTIAL_INT because
+ turning vector or complex modes into shifts causes problems. */
+
+ if (GET_MODE_CLASS (GET_MODE (XEXP (x, 0))) != MODE_INT
+ && GET_MODE_CLASS (GET_MODE (XEXP (x, 0))) != MODE_PARTIAL_INT)
+ return x;
+
len = GET_MODE_BITSIZE (GET_MODE (XEXP (x, 0)));
/* If the inner object has VOIDmode (the only way this can happen
is if it is an ASM_OPERANDS), we can't do anything since we don't
*************** expand_compound_operation (x)
*** 5655,5660 ****
--- 5662,5674 ----
|| GET_MODE (XEXP (x, 0)) == VOIDmode)
return x;
+ /* Reject MODEs that aren't MODE_INT or MODE_PARTIAL_INT because
+ turning vector or complex modes into shifts causes problems. */
+
+ if (GET_MODE_CLASS (GET_MODE (XEXP (x, 0))) != MODE_INT
+ && GET_MODE_CLASS (GET_MODE (XEXP (x, 0))) != MODE_PARTIAL_INT)
+ return x;
+
len = INTVAL (XEXP (x, 1));
pos = INTVAL (XEXP (x, 2));
*************** expand_field_assignment (x)
*** 5862,5873 ****
compute_mode = GET_MODE (inner);
! /* Don't attempt bitwise arithmetic on non-integral modes. */
! if (! INTEGRAL_MODE_P (compute_mode))
{
enum machine_mode imode;
! /* Something is probably seriously wrong if this matches. */
if (! FLOAT_MODE_P (compute_mode))
break;
--- 5876,5889 ----
compute_mode = GET_MODE (inner);
! /* Don't attempt bitwise arithmetic on non-integer modes. */
! if (GET_MODE_CLASS (compute_mode) != MODE_INT
! && GET_MODE_CLASS (compute_mode) != MODE_PARTIAL_INT)
!
{
enum machine_mode imode;
! /* Don't try doing anything for Vector or Integer complex types. */
if (! FLOAT_MODE_P (compute_mode))
break;
*************** simplify_comparison (code, pop0, pop1)
*** 10177,10183 ****
/* Get the constant we are comparing against and turn off all bits
not on in our mode. */
! const_op = trunc_int_for_mode (INTVAL (op1), mode);
op1 = GEN_INT (const_op);
/* If we are comparing against a constant power of two and the value
--- 10193,10201 ----
/* Get the constant we are comparing against and turn off all bits
not on in our mode. */
! const_op = INTVAL (op1);
! if (mode != VOIDmode)
! const_op = trunc_int_for_mode (const_op, mode);
op1 = GEN_INT (const_op);
/* If we are comparing against a constant power of two and the value
*** gcc.orig/gcc/recog.c Fri Aug 16 12:12:21 2002
--- gcc/gcc/recog.c Fri Aug 16 12:12:55 2002
*************** general_operand (op, mode)
*** 954,959 ****
--- 954,960 ----
return 0;
if (GET_CODE (op) == CONST_INT
+ && mode != VOIDmode
&& trunc_int_for_mode (INTVAL (op), mode) != INTVAL (op))
return 0;
*************** immediate_operand (op, mode)
*** 1159,1164 ****
--- 1160,1166 ----
return 0;
if (GET_CODE (op) == CONST_INT
+ && mode != VOIDmode
&& trunc_int_for_mode (INTVAL (op), mode) != INTVAL (op))
return 0;
*************** nonmemory_operand (op, mode)
*** 1241,1246 ****
--- 1243,1249 ----
return 0;
if (GET_CODE (op) == CONST_INT
+ && mode != VOIDmode
&& trunc_int_for_mode (INTVAL (op), mode) != INTVAL (op))
return 0;
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: patch to stop GCC from shifting by a negative number
2002-08-16 14:09 Steve Ellcey
@ 2002-08-17 10:49 ` Richard Henderson
0 siblings, 0 replies; 7+ messages in thread
From: Richard Henderson @ 2002-08-17 10:49 UTC (permalink / raw)
To: Steve Ellcey; +Cc: geoffk, gcc-patches
On Fri, Aug 16, 2002 at 02:08:47PM -0700, Steve Ellcey wrote:
> - /* Don't attempt bitwise arithmetic on non-integral modes. */
> - if (! INTEGRAL_MODE_P (compute_mode))
> + /* Don't attempt bitwise arithmetic on non-integer modes. */
> + if (GET_MODE_CLASS (compute_mode) != MODE_INT
> + && GET_MODE_CLASS (compute_mode) != MODE_PARTIAL_INT)
> +
> {
Please introduce a new macro SCALAR_INT_MODE_P for this test,
and use as needed throughout. Do not introduce random whitespace.
Ok with that change.
r~
^ permalink raw reply [flat|nested] 7+ messages in thread
* patch to stop GCC from shifting by a negative number
@ 2002-08-12 15:57 Steve Ellcey
2002-08-12 16:33 ` Richard Henderson
2002-08-12 16:56 ` Geoff Keating
0 siblings, 2 replies; 7+ messages in thread
From: Steve Ellcey @ 2002-08-12 15:57 UTC (permalink / raw)
To: gcc-patches
I tracked down a difference between a cross-compiling GCC and a native
built GCC to trunc_int_for_mode trying to shift by a negative number
when the function is getting called by general_operand with a CONST_INT
rtx and a mode of VOIDmode.
This seems like legal input but I found if mode == VOIDmode then
GET_MODE_BITSIZE returns 0 and width is set to 0 in trunc_int_for_mode,
this leads us to to try and shift sign by -1 and this is undefined
behaviour according to the C standard. In one case I came out of
trunc_int_for_mode with my original constant (and things were good) and
in the other I came out with zero (causing a compile time failure).
I haven't included a test because I don't have one that fails natively,
only when built as a cross compiler from HP-UX PA to HP-UX IA64.
I am rebuilding and retesting now, OK to check in if all the tests pass?
Steve Ellcey
sje@cup.hp.com
2002-08-12 Steve Ellcey <sje@cup.hp.com>
* gcc/explow.c (trunc_int_for_mode): Do not try to shift
by a negative number.
*** gcc.orig/gcc/explow.c Mon Aug 12 15:35:33 2002
--- gcc/gcc/explow.c Mon Aug 12 15:36:06 2002
*************** trunc_int_for_mode (c, mode)
*** 54,60 ****
/* Sign-extend for the requested mode. */
! if (width < HOST_BITS_PER_WIDE_INT)
{
HOST_WIDE_INT sign = 1;
sign <<= width - 1;
--- 54,60 ----
/* Sign-extend for the requested mode. */
! if (width > 0 && width < HOST_BITS_PER_WIDE_INT)
{
HOST_WIDE_INT sign = 1;
sign <<= width - 1;
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: patch to stop GCC from shifting by a negative number
2002-08-12 15:57 Steve Ellcey
@ 2002-08-12 16:33 ` Richard Henderson
2002-08-12 16:56 ` Geoff Keating
1 sibling, 0 replies; 7+ messages in thread
From: Richard Henderson @ 2002-08-12 16:33 UTC (permalink / raw)
To: Steve Ellcey; +Cc: gcc-patches
On Mon, Aug 12, 2002 at 03:57:10PM -0700, Steve Ellcey wrote:
> - if (width < HOST_BITS_PER_WIDE_INT)
> + if (width > 0 && width < HOST_BITS_PER_WIDE_INT)
Not ok. This implies that we're using trunc_int_for_mode
with a VOIDmode or BLKmode argument. This is wrong.
Feel free to add an abort for this case here.
r~
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: patch to stop GCC from shifting by a negative number
2002-08-12 15:57 Steve Ellcey
2002-08-12 16:33 ` Richard Henderson
@ 2002-08-12 16:56 ` Geoff Keating
1 sibling, 0 replies; 7+ messages in thread
From: Geoff Keating @ 2002-08-12 16:56 UTC (permalink / raw)
To: sje; +Cc: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 533 bytes --]
Steve Ellcey <sje@cup.hp.com> writes:
> I tracked down a difference between a cross-compiling GCC and a native
> built GCC to trunc_int_for_mode trying to shift by a negative number
> when the function is getting called by general_operand with a CONST_INT
> rtx and a mode of VOIDmode.
Ugh. It really shouldn't be called this way; it makes no sense.
In fact, in const_int_operand, there's a guard around the
trunc_int_for_mode call to check it against VOIDmode.
What do you think about a patch like this (very lightly tested)?
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: gcc-notruncvoidformode.patch --]
[-- Type: text/x-patch, Size: 2544 bytes --]
2002-08-12 Geoffrey Keating <geoffk@redhat.com>
* explow.c (trunc_int_for_mode): Abort when the mode is not
an integer mode.
* combine.c (simplify_comparison): Don't call trunc_int_for_mode
for VOIDmode.
* recog.c (general_operand): Likewise.
(immediate_operand): Likewise.
(nonmemory_operand): Likewise.
Index: combine.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/combine.c,v
retrieving revision 1.309
diff -p -u -p -r1.309 combine.c
--- combine.c 1 Aug 2002 09:08:38 -0000 1.309
+++ combine.c 12 Aug 2002 23:52:14 -0000
@@ -10177,7 +10177,9 @@ simplify_comparison (code, pop0, pop1)
/* Get the constant we are comparing against and turn off all bits
not on in our mode. */
- const_op = trunc_int_for_mode (INTVAL (op1), mode);
+ const_op = INTVAL (op1);
+ if (mode != VOIDmode)
+ const_op = trunc_int_for_mode (const_op, mode);
op1 = GEN_INT (const_op);
/* If we are comparing against a constant power of two and the value
Index: explow.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/explow.c,v
retrieving revision 1.99
diff -p -u -p -r1.99 explow.c
--- explow.c 3 Aug 2002 20:20:35 -0000 1.99
+++ explow.c 12 Aug 2002 23:52:15 -0000
@@ -49,6 +49,11 @@ trunc_int_for_mode (c, mode)
{
int width = GET_MODE_BITSIZE (mode);
+ /* You want to truncate to a _what_? */
+ if (GET_MODE_CLASS (mode) != MODE_INT
+ && GET_MODE_CLASS (mode) != MODE_PARTIAL_INT)
+ abort ();
+
/* Canonicalize BImode to 0 and STORE_FLAG_VALUE. */
if (mode == BImode)
return c & 1 ? STORE_FLAG_VALUE : 0;
Index: recog.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/recog.c,v
retrieving revision 1.162
diff -p -u -p -r1.162 recog.c
--- recog.c 23 Jul 2002 20:50:59 -0000 1.162
+++ recog.c 12 Aug 2002 23:52:15 -0000
@@ -954,6 +954,7 @@ general_operand (op, mode)
return 0;
if (GET_CODE (op) == CONST_INT
+ && mode != VOIDmode
&& trunc_int_for_mode (INTVAL (op), mode) != INTVAL (op))
return 0;
@@ -1159,6 +1160,7 @@ immediate_operand (op, mode)
return 0;
if (GET_CODE (op) == CONST_INT
+ && mode != VOIDmode
&& trunc_int_for_mode (INTVAL (op), mode) != INTVAL (op))
return 0;
@@ -1241,6 +1243,7 @@ nonmemory_operand (op, mode)
return 0;
if (GET_CODE (op) == CONST_INT
+ && mode != VOIDmode
&& trunc_int_for_mode (INTVAL (op), mode) != INTVAL (op))
return 0;
[-- Attachment #3: Type: text/plain, Size: 64 bytes --]
--
- Geoffrey Keating <geoffk@geoffk.org> <geoffk@redhat.com>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2002-08-17 17:49 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2002-08-13 12:46 patch to stop GCC from shifting by a negative number Steve Ellcey
2002-08-13 12:56 ` Geoff Keating
-- strict thread matches above, loose matches on Subject: below --
2002-08-16 14:09 Steve Ellcey
2002-08-17 10:49 ` Richard Henderson
2002-08-12 15:57 Steve Ellcey
2002-08-12 16:33 ` Richard Henderson
2002-08-12 16:56 ` Geoff Keating
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).