On Mar 17, 2012, at 12:37 AM, Richard Sandiford wrote:
> Mike Stump <mikest...@comcast.net> writes:
>> This removes some wrong code.
>> 
>> Ok?
>> 
>> Index: gcc/emit-rtl.c
>> ===================================================================
>> --- gcc/emit-rtl.c      (revision 184563)
>> +++ gcc/emit-rtl.c      (working copy)
>> @@ -540,8 +540,6 @@ immed_double_const (HOST_WIDE_INT i0, HO
>> 
>>       if (GET_MODE_BITSIZE (mode) <= HOST_BITS_PER_WIDE_INT)
>>        return gen_int_mode (i0, mode);
>> -
>> -      gcc_assert (GET_MODE_BITSIZE (mode) == 2 * HOST_BITS_PER_WIDE_INT);
>>     }
>> 
>>   /* If this integer fits in one word, return a CONST_INT.  */
> 
> Is this because you have an integer mode between HOST_BITS_PER_WIDE_INT and
> 2 * HOST_BITS_PER_WIDE_INT?

Yes, I have those, but, that wasn't the testcase I had in mind.

> Or is this because you have an integer mode wider than
> 2*OST_BITS_PER_WIDE_INT?

Yes.

> We currently only support constant integer
> widths <= 2*HOST_BITS_PER_WIDE_INT, and the assert is correctly
> triggering if we try to build a wider constant.

Can you give me a concrete example of what will fail with the constant 0 
generated by:

        return GEN_INT (i0);

when the mode is 2*HOST_BITS_PER_WIDE_INT?  When the mode is larger than this?

If you cannot, can you refer me to documentation where this is discussed?  If 
not, how about code?

What I am seeing is that it works and generates correct code without the 
assert.  My contention is that any code that fails to work, is buggy, and 
should be fixed, and once fixed, then there is no code that doesn't work, and 
hence the assert is wrong.  If you want to argue that the code that fails, 
isn't buggy, go ahead, I'd love to hear it.

See, we already shorten the width of wide constants and expect that to work.  
This is just another instance of shortening.  Now, just to see what lurks in 
there, I when through 100% of the uses of CONST_DOUBLE in *.c to get a feel for 
what might go wrong, the code is as benign as I expected, every instance I saw, 
looked reasonably safe.  It doesn't get any better than this.

> Obviously it'd be nice

Yes, but that is quite a lot of work.  It also happens to be orthogonal to the 
immediate issue at hand.

> if we supported arbitrary widths, e.g. by replacing CONST_INT and
> CONST_DOUBLE with a single n-HOST_WIDE_INT rtx (and immed_double_const
> with some kind of nary builder, etc.).  It won't be easy though.
> 
> Removing the assert seems like papering over the problem.

Do you have an example of a failure?  Hint, without a failure, there is no bug, 
I cannot fix a bug, when there is no bug.

> FWIW, here's another case where this came up:
> 
>    http://gcc.gnu.org/ml/gcc-patches/2011-03/msg01220.html

Yes, and surprisingly, or not it is the exact same case as I can into.  Notice 
nowhere in that bug or thread, is _any_ detailed discussed as to why that would 
be a bad fix.

So, I'm looking for approval, or a concrete reason why it is a bad idea.

Reply via email to