Hi,
On Tue, 20 Mar 2012, Mike Stump wrote:
> > Actually you did. I've tried yesterday to come up with a text that
> > would do the same (because I agree with you that deleting the assert
> > changes the spec of the function,
>
> The spec of the function is the text above the definition of the
> function, coupled with the information in the .texi file, would you
> agree?
Actually, I wouldn't. The real spec includes many pieces of information,
the comments (that can be incomplete or become out of date), the .texi
docu (which can be even more incomplete and out of date), the code (which
can conflict with the comments and still be the correct variant) and the
current usage (which can conflict with everything of the above). asserts
are IMO even a nice way of documenting parts of the spec.
> If so, could you please quote the text of the spec which would
> be violated by removing the assert? Could you please give a specific
> value with which we could talk about that shows a violation of the spec.
Richard did so. If the high bit of i1 is set then currently you will get
a negative number produced no matter the absolute value of it. That's IMO
part of the spec, high bit set --> negative number. negative as defined
by the various routines dealing with CONST_INT or CONST_DOUBLE interpreted
in the modes allowed for creating them.
If you were to allow modes of larger size than what could be completely
specified with the two HWI arguments i0 and i1, then it suddenly depends
on the absolute value if you get a negative or positive number. For small
negative numbers (those for which i1 is ~0 and i0 < 0) you'll still get a
negative CONST_INT. For large negative numbers you'll get a CONST_DOUBLE,
that when interpreted in the large requested mode (which is the only thing
that makes sense) is positive. It doesn't matter that it's still
negative when interpreted in a smaller mode.
Hence all values where i1 is between (HWI)1 << (hwibits-1)) and
((HWI)~0)-1 are the values you're searching for, that show the problem.
As you correctly note the routine will of course generate the exact same
CONST_DOUBLE object no matter the mode given, but they have to be
interpreted together with the given mode.
This positive/negative inconsistency doesn't make sense to allow, and the
assert ensures that it isn't allowed.
Now, this inconsistency can also be avoided via different means. By
extending the block comment in front of the function for instance, but
then the assert would make even more sense. So Richards proposal to move
the assert is better: The problem occurs only with large positive or
negative values (i1 not 0 or ~0), so the mode-size check can be moved
after the GEN_INT call.
This would have the seemingly strange effect of disallowing too large
modes only for large values, but that's simply a side-effect of
CONST_DOUBLE and the whole associated machinery not being able to
consistently deal with constants wider than 2*HWI_BITS.
> My position is simple, the spec is what is above the definition and the
> .texi files, and the stuff inside the definition are interesting
> implementation details of that spec, which _never_ modify the spec.
As an abstract goal that's good. But reality is that this isn't the case.
GCC is quite excellently commented, but it doesn't fit the ideal. Using
the fact that it isn't ideal to claim that the spec doesn't say anything
about large modes (when the assert clearly disallows them) is absurd.
> My position is that 0 is a value which the spec defines, and for which
> we assert. Please quote the line from the spec that defines what we do
> in that case. I've never seen anyone quote such a line. To support
> your position, I will insist on a direct quote from the spec.
This line disallows the value 0 with large modes:
gcc_assert (GET_MODE_BITSIZE (mode) == 2 * HOST_BITS_PER_WIDE_INT);
I insist on it being part of the spec. Moving the assert changes the spec
to allow 0 (and generally small positive and negative numbers) to also be
generated for larger modes. If you so want to change the spec nobody
would be opposed.
> if I is 42, we abort. To back the position that spec must not be
> changed, you need to explain at least one thing for which the wrong
> thing will happen if the spec did change. If you want to go down that
> path, you will need to furnish one example where badness happens with 0,
> not 2, not 3, but 0.
Huh. Removing the assert wouldn't only allow 0, but also other values.
I don't understand your argumentation: "because for 0 nothing bad happens,
that proves that nothing bad happens for any other values which we would
also allow, hence I can remove the assert"? It of course doesn't prove
anything at all. In any case, above I have given the values that will be
problematic (and they don't include 0), and a way of changing the spec to
disallow only them, instead of all values. Actually Richard S. did so, I
just repeated him.
Ciao,
Michael.
