On Mar 19, 2012, at 2:44 PM, Richard Sandiford wrote:
> Mike Stump <mikest...@comcast.net> writes:
>>> If we're going to remove the assert, we need to define stuff like
>>> that.
>> Orthogonal.  The rest of the compiler defines what happens, it either
>> is inconsistent, in which case it is by fiat, undefined, or it is
>> consistent, in which case that consistency defines it.  The compiler
>> is free to document this in a nice way, or do, what is usually done,
>> which is to assume everybody just knows what it does.  Anyway, my
>> point is, this routine doesn't define the data structure, and is
>> _completely_ orthogonal to your concern.  It doesn't matter if it zero
>> extends or sign extends or is inconsistent, has bugs, doesn't have
>> bugs, is documented, or isn't documented.  In every single one of
>> these cases, the code in the routine I am fixing, doesn't change.
>> That is _why_ it is orthogonal.  If it weren't, you'd be able to state
>> a value for which is mattered.  You can't, which is why you are wrong.
>> If you think you are not wrong, please state a value for which it
>> matters how it is defined.
> immed_double_const and CONST_DOUBLE are currently
> only defined for 2 HOST_WIDE_INTs.

I don't happen to share your view.  The routine is defined by documentation.  
The documentation might exist in a .texi file, in this case there is no texi 
file for immed_double_const I don't think, next up, it is defined by the 
comments before the routine.  In this case, it isn't so defined.

The current definition reads:

/* Return a CONST_DOUBLE or CONST_INT for a value specified as a pair           
   of ints: I0 is the low-order word and I1 is the high-order word.             
   Do not use this routine for non-integer modes; convert to                    

which, is is fine, and I don't _want_ to change that definition of the routine. 
 I can't fix it, because it isn't broken.  If it were, you would be able to 
state a case where the new code behaves in a manor inconsistent with the 
definition, since there is none you cannot state one, and this is _why_ you 
have failed to state such a case.  If you disagree, please state the case.

Now, if you review comment is, could you please update the comments in the 
routine, I would just say, oh, sure:

Index: emit-rtl.c
--- emit-rtl.c  (revision 184563)
+++ emit-rtl.c  (working copy)
@@ -525,10 +525,9 @@ immed_double_const (HOST_WIDE_INT i0, HO
      1) If GET_MODE_BITSIZE (mode) <= HOST_BITS_PER_WIDE_INT, then we use
-     2) GET_MODE_BITSIZE (mode) == 2 * HOST_BITS_PER_WIDE_INT, but the value of
-       the integer fits into HOST_WIDE_INT anyway (i.e., i1 consists only
-       from copies of the sign bit, and sign of i0 and i1 are the same),  then
-       we return a CONST_INT for i0.
+     2) If the value of the integer fits into HOST_WIDE_INT anyway
+       (i.e., i1 consists only from copies of the sign bit, and sign
+       of i0 and i1 are the same), then we return a CONST_INT for i0.
      3) Otherwise, we create a CONST_DOUBLE for i0 and i1.  */
   if (mode != VOIDmode)
@@ -540,8 +539,6 @@ immed_double_const (HOST_WIDE_INT i0, HO
        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.  */

Sorry I missed it.  Now, on to CONST_DOUBLE.  It does appear in a texi file:

@findex const_double
@item (const_double:@var{m} @var{i0} @var{i1} @dots{})
Represents either a floating-point constant of mode @var{m} or an
integer constant too large to fit into @code{HOST_BITS_PER_WIDE_INT}
bits but small enough to fit within twice that number of bits (GCC
does not provide a mechanism to represent even larger constants).  In
the latter case, @var{m} will be @code{VOIDmode}.

If @var{m} is @code{VOIDmode}, the bits of the value are stored in
@var{i0} and @var{i1}.  @var{i0} is customarily accessed with the macro
@code{CONST_DOUBLE_LOW} and @var{i1} with @code{CONST_DOUBLE_HIGH}.

Here again, I don't want to change the definition.  The current definition 
applies and I am merely making the code conform to it.  It says that 
CONST_DOUBLE is used when the _value_ of the constant is too large to fit into 

So, if you disagree with me, you will necessarily have to quote the definition 
you are using, explain what the words mean to you _and_ state a specific case 
in which the code post modification doesn't not conform with the existing 
definition.  You have failed yet again to do that.

> So, as good functions do, immed_double_const asserts that it is not being 
> used out of spec.

This does not follow from the definition.  0 is a value that fits into 
HOST_BITS_PER_WIDE_INT bits.  It is representable in 0 bits.  
HOST_BITS_PER_WIDE_INT is zero or more, and by induction, is representable by 

> You want to remove that restriction on immed_double_const and CONST_DOUBLE.
> That is, you want to change their spec.  We should only do that if we define
> what the new semantics are.

You're assuming a definition for CONST_DOUBLE that only exists in your mind, 
instead, please refer to the actual definition in the .texi file.

Reply via email to