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.


Reply via email to