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.