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
REAL_VALUE_TYPE and use CONST_DOUBLE_FROM_REAL_VALUE. */
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
gen_int_mode.
- 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
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. */
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}.
@findex CONST_DOUBLE_LOW
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
HOST_BITS_PER_WIDE_INT bits.
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
HOST_BITS_PER_WIDE_INT bits.
> 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.
