Richard Biener <[email protected]> writes:
> On Sat, Nov 2, 2013 at 3:25 PM, Richard Sandiford
> <[email protected]> wrote:
>> Kenneth Zadeck <[email protected]> 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 +0000
>> +++ gcc/rtl.h 2013-11-02 14:22:05.636007860 +0000
>> @@ -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 +0000
+++ gcc/rtl.h 2013-11-06 22:02:08.199889647 +0000
@@ -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);
