Patchwork [wide-int] Do not treat rtxes as sign-extended

login
register
mail settings
Submitter Richard Sandiford
Date Nov. 2, 2013, 10:30 a.m.
Message ID <874n7vkse3.fsf@talisman.default>
Download mbox | patch
Permalink /patch/287969/
State New
Headers show

Comments

Richard Sandiford - Nov. 2, 2013, 10:30 a.m.
Bah.  After all that effort, it turns out that -- by design --
there is one special case where CONST_INTs are not sign-extended.
Nonzero/true BImode integers are stored as STORE_FLAG_VALUE,
which can be 1 rather than -1.  So (const_int 1) can be a valid
BImode integer -- and consequently (const_int -1) can be wrong --
even though BImode only has 1 bit.

It might be nice to change that, but for wide-int I think we should
just treat rtxes like trees for now.

Tested on powerpc64-linux-gnu and x86_64-linux-gnu.  It fixes some ICEs
seen on bfin-elf.  OK to install?

Thanks,
Richard
Mike Stump - Nov. 2, 2013, 10:38 a.m.
On Nov 2, 2013, at 3:30 AM, Richard Sandiford <rdsandiford@googlemail.com> wrote:
> Bah.  After all that effort, it turns out that -- by design --
> there is one special case where CONST_INTs are not sign-extended.

I'm thinking this needs commentary in wide-int.h, though, not sure what we'd say...
Richard Sandiford - Nov. 2, 2013, 11:26 a.m.
Mike Stump <mikestump@comcast.net> writes:
> On Nov 2, 2013, at 3:30 AM, Richard Sandiford
> <rdsandiford@googlemail.com> wrote:
>> Bah.  After all that effort, it turns out that -- by design --
>> there is one special case where CONST_INTs are not sign-extended.
>
> I'm thinking this needs commentary in wide-int.h, though, not sure what
> we'd say...

AIUI the only valid values of STORE_FLAG_VALUE are 1 and -1.  If others
are used, the rtx decompose routine would need to use the scratch array.

So wide-int itself shouldn't need to care.  It just means that a
1-precision rtx input can be sign or zero extended, just like for
N-precision trees.  And we indicate that by setting is_sign_extended
to false.

Thanks,
Richard
Kenneth Zadeck - Nov. 2, 2013, 1:45 p.m.
On 11/02/2013 06:30 AM, Richard Sandiford wrote:
> Bah.  After all that effort, it turns out that -- by design --
> there is one special case where CONST_INTs are not sign-extended.
> Nonzero/true BImode integers are stored as STORE_FLAG_VALUE,
> which can be 1 rather than -1.  So (const_int 1) can be a valid
> BImode integer -- and consequently (const_int -1) can be wrong --
> even though BImode only has 1 bit.
>
> It might be nice to change that, but for wide-int I think we should
> just treat rtxes like trees for now.
>
> Tested on powerpc64-linux-gnu and x86_64-linux-gnu.  It fixes some ICEs
> seen on bfin-elf.  OK to install?
do we have to throw away the baby with the bath water here?  I guess 
what you are saying is that it is worse to have is_sign_extended be a 
variable that is almost always true than to be a hard false.

also we could preserve the test and make it not apply to bimode.

kenny


> Thanks,
> Richard
>
>
> Index: gcc/rtl.h
> ===================================================================
> --- gcc/rtl.h	(revision 204311)
> +++ gcc/rtl.h	(working copy)
> @@ -1408,7 +1408,9 @@
>     {
>       static const enum precision_type precision_type = VAR_PRECISION;
>       static const bool host_dependent_precision = false;
> -    static const bool is_sign_extended = true;
> +    /* This ought to be true, except for the special case that BImode
> +       is canonicalized to STORE_FLAG_VALUE, which might be 1.  */
> +    static const bool is_sign_extended = false;
>       static unsigned int get_precision (const rtx_mode_t &);
>       static wi::storage_ref decompose (HOST_WIDE_INT *, unsigned int,
>   				      const rtx_mode_t &);
> @@ -1430,10 +1432,6 @@
>     switch (GET_CODE (x.first))
>       {
>       case CONST_INT:
> -      if (precision < HOST_BITS_PER_WIDE_INT)
> -	gcc_checking_assert (INTVAL (x.first)
> -			     == sext_hwi (INTVAL (x.first), precision));
> -
>         return wi::storage_ref (&INTVAL (x.first), 1, precision);
>         
>       case CONST_WIDE_INT:

Patch

Index: gcc/rtl.h
===================================================================
--- gcc/rtl.h	(revision 204311)
+++ gcc/rtl.h	(working copy)
@@ -1408,7 +1408,9 @@ 
   {
     static const enum precision_type precision_type = VAR_PRECISION;
     static const bool host_dependent_precision = false;
-    static const bool is_sign_extended = true;
+    /* This ought to be true, except for the special case that BImode
+       is canonicalized to STORE_FLAG_VALUE, which might be 1.  */
+    static const bool is_sign_extended = false;
     static unsigned int get_precision (const rtx_mode_t &);
     static wi::storage_ref decompose (HOST_WIDE_INT *, unsigned int,
 				      const rtx_mode_t &);
@@ -1430,10 +1432,6 @@ 
   switch (GET_CODE (x.first))
     {
     case CONST_INT:
-      if (precision < HOST_BITS_PER_WIDE_INT)
-	gcc_checking_assert (INTVAL (x.first)
-			     == sext_hwi (INTVAL (x.first), precision));
-
       return wi::storage_ref (&INTVAL (x.first), 1, precision);
       
     case CONST_WIDE_INT: