Re: [wide-int] Do not treat rtxes as sign-extended
Richard Biener richard.guent...@gmail.com writes:
On Sat, Nov 2, 2013 at 3:25 PM, Richard Sandiford
rdsandif...@googlemail.com wrote:
Kenneth Zadeck zad...@naturalbridge.com writes:
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.
Right. is_sign_extended is only useful if it's a compile-time value.
Making it a run-time value would negate the benefit.
I think in practice STORE_FLAG_VALUE is a compile-time constant too,
so we could set is_sign_extended to STORE_FLAG_VALUE == -1. But AFAICT
that would only help SPU and m68k.
also we could preserve the test and make it not apply to bimode.
You mean the one in the assert? Yeah, OK. How about this version?
Richard
Index: gcc/rtl.h
===
--- gcc/rtl.h 2013-11-02 11:06:12.738517644 +
+++ gcc/rtl.h 2013-11-02 14:22:05.636007860 +
@@ -1408,7 +1408,9 @@ typedef std::pair rtx, enum machine_mod
{
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 );
@@ -1432,7 +1434,8 @@ wi::int_traits rtx_mode_t::decompose (
case CONST_INT:
if (precision HOST_BITS_PER_WIDE_INT)
gcc_checking_assert (INTVAL (x.first)
-== sext_hwi (INTVAL (x.first), precision));
+== sext_hwi (INTVAL (x.first), precision)
+|| (precision == 1 INTVAL (x.first) == 1));
please add a comment here (and a check for BImode?).
OK, done as follows.
Thanks,
Richard
Index: gcc/rtl.h
===
--- gcc/rtl.h 2013-11-06 21:53:29.013756409 +
+++ gcc/rtl.h 2013-11-06 22:02:08.199889647 +
@@ -1433,9 +1433,11 @@ wi::int_traits rtx_mode_t::decompose (
{
case CONST_INT:
if (precision HOST_BITS_PER_WIDE_INT)
+ /* Nonzero BImodes are stored as STORE_FLAG_VALUE, which on many
+ targets is 1 rather than -1. */
gcc_checking_assert (INTVAL (x.first)
== sext_hwi (INTVAL (x.first), precision)
-|| (precision == 1 INTVAL (x.first) == 1));
+|| (x.second == BImode INTVAL (x.first) == 1));
return wi::storage_ref (INTVAL (x.first), 1, precision);
Re: [wide-int] Do not treat rtxes as sign-extended
On Sat, Nov 2, 2013 at 3:25 PM, Richard Sandiford
rdsandif...@googlemail.com wrote:
Kenneth Zadeck zad...@naturalbridge.com writes:
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.
Right. is_sign_extended is only useful if it's a compile-time value.
Making it a run-time value would negate the benefit.
I think in practice STORE_FLAG_VALUE is a compile-time constant too,
so we could set is_sign_extended to STORE_FLAG_VALUE == -1. But AFAICT
that would only help SPU and m68k.
also we could preserve the test and make it not apply to bimode.
You mean the one in the assert? Yeah, OK. How about this version?
Richard
Index: gcc/rtl.h
===
--- gcc/rtl.h 2013-11-02 11:06:12.738517644 +
+++ gcc/rtl.h 2013-11-02 14:22:05.636007860 +
@@ -1408,7 +1408,9 @@ typedef std::pair rtx, enum machine_mod
{
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 );
@@ -1432,7 +1434,8 @@ wi::int_traits rtx_mode_t::decompose (
case CONST_INT:
if (precision HOST_BITS_PER_WIDE_INT)
gcc_checking_assert (INTVAL (x.first)
-== sext_hwi (INTVAL (x.first), precision));
+== sext_hwi (INTVAL (x.first), precision)
+|| (precision == 1 INTVAL (x.first) == 1));
please add a comment here (and a check for BImode?).
Thanks,
Richard.
return wi::storage_ref (INTVAL (x.first), 1, precision);
Re: [wide-int] Do not treat rtxes as sign-extended
On Nov 2, 2013, at 3:30 AM, Richard Sandiford rdsandif...@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...
Re: [wide-int] Do not treat rtxes as sign-extended
Mike Stump mikest...@comcast.net writes: On Nov 2, 2013, at 3:30 AM, Richard Sandiford rdsandif...@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
Re: [wide-int] Do not treat rtxes as sign-extended
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:
Re: [wide-int] Do not treat rtxes as sign-extended
Kenneth Zadeck zad...@naturalbridge.com writes:
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.
Right. is_sign_extended is only useful if it's a compile-time value.
Making it a run-time value would negate the benefit.
I think in practice STORE_FLAG_VALUE is a compile-time constant too,
so we could set is_sign_extended to STORE_FLAG_VALUE == -1. But AFAICT
that would only help SPU and m68k.
also we could preserve the test and make it not apply to bimode.
You mean the one in the assert? Yeah, OK. How about this version?
Richard
Index: gcc/rtl.h
===
--- gcc/rtl.h 2013-11-02 11:06:12.738517644 +
+++ gcc/rtl.h 2013-11-02 14:22:05.636007860 +
@@ -1408,7 +1408,9 @@ typedef std::pair rtx, enum machine_mod
{
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 );
@@ -1432,7 +1434,8 @@ wi::int_traits rtx_mode_t::decompose (
case CONST_INT:
if (precision HOST_BITS_PER_WIDE_INT)
gcc_checking_assert (INTVAL (x.first)
-== sext_hwi (INTVAL (x.first), precision));
+== sext_hwi (INTVAL (x.first), precision)
+|| (precision == 1 INTVAL (x.first) == 1));
return wi::storage_ref (INTVAL (x.first), 1, precision);
Re: [wide-int] Do not treat rtxes as sign-extended
On 11/02/2013 10:25 AM, Richard Sandiford wrote:
Kenneth Zadeck zad...@naturalbridge.com writes:
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.
Right. is_sign_extended is only useful if it's a compile-time value.
Making it a run-time value would negate the benefit.
I think in practice STORE_FLAG_VALUE is a compile-time constant too,
so we could set is_sign_extended to STORE_FLAG_VALUE == -1. But AFAICT
that would only help SPU and m68k.
also we could preserve the test and make it not apply to bimode.
You mean the one in the assert? Yeah, OK. How about this version?
Richard
Index: gcc/rtl.h
===
--- gcc/rtl.h 2013-11-02 11:06:12.738517644 +
+++ gcc/rtl.h 2013-11-02 14:22:05.636007860 +
@@ -1408,7 +1408,9 @@ typedef std::pair rtx, enum machine_mod
{
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 );
@@ -1432,7 +1434,8 @@ wi::int_traits rtx_mode_t::decompose (
case CONST_INT:
if (precision HOST_BITS_PER_WIDE_INT)
gcc_checking_assert (INTVAL (x.first)
-== sext_hwi (INTVAL (x.first), precision));
+== sext_hwi (INTVAL (x.first), precision)
+|| (precision == 1 INTVAL (x.first) == 1));
return wi::storage_ref (INTVAL (x.first), 1, precision);
yes, this is fine.
kenny
