From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 17667 invoked by alias); 19 Mar 2012 23:31:33 -0000 Received: (qmail 17654 invoked by uid 22791); 19 Mar 2012 23:31:31 -0000 X-SWARE-Spam-Status: No, hits=-1.9 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_NONE,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from qmta03.emeryville.ca.mail.comcast.net (HELO qmta03.emeryville.ca.mail.comcast.net) (76.96.30.32) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 19 Mar 2012 23:31:06 +0000 Received: from omta16.emeryville.ca.mail.comcast.net ([76.96.30.72]) by qmta03.emeryville.ca.mail.comcast.net with comcast id nbUc1i0021ZMdJ4A3bX5wC; Mon, 19 Mar 2012 23:31:05 +0000 Received: from up.mrs.kithrup.com ([24.4.193.8]) by omta16.emeryville.ca.mail.comcast.net with comcast id nbX41i00S0BKwT48cbX5nm; Mon, 19 Mar 2012 23:31:05 +0000 Subject: Re: remove wrong code in immed_double_const Mime-Version: 1.0 (Apple Message framework v1084) Content-Type: text/plain; charset=us-ascii From: Mike Stump In-Reply-To: <8762e09sgc.fsf@talisman.home> Date: Mon, 19 Mar 2012 23:31:00 -0000 Cc: gcc-patches Patches Content-Transfer-Encoding: quoted-printable Message-Id: <0A5CBD0C-FC94-4637-B230-1A83372DE91A@comcast.net> References: <5FF5A724-3FE1-4E97-8124-542A0B8259FE@comcast.net> <87obrvd6fh.fsf@talisman.home> <87haxmgqoo.fsf@talisman.home> <7C6A7462-C1D3-4765-83FF-3B3C726D92E5@comcast.net> <8762e09sgc.fsf@talisman.home> To: Richard Sandiford X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org X-SW-Source: 2012-03/txt/msg01318.txt.bz2 On Mar 19, 2012, at 2:44 PM, Richard Sandiford wrote: > Mike Stump writes: >>> If we're going to remove the assert, we need to define stuff like >>> that. >>=20 >> 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. >=20 > 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 t= he 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=20=20= =20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20= =20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20= =20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20= =20=20=20=20=20=20=20=20=20=20=20 of ints: I0 is the low-order word and I1 is the high-order word.=20=20= =20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20= =20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20= =20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20= =20=20=20=20=20=20=20=20=20=20=20=20=20 Do not use this routine for non-integer modes; convert to=20=20=20=20=20= =20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20= =20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20= =20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20= =20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20 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 rout= ine. I can't fix it, because it isn't broken. If it were, you would be ab= le 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 c= ase. 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 =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D --- emit-rtl.c (revision 184563) +++ emit-rtl.c (working copy) @@ -525,10 +525,9 @@ immed_double_const (HOST_WIDE_INT i0, HO =20 1) If GET_MODE_BITSIZE (mode) <=3D HOST_BITS_PER_WIDE_INT, then we use gen_int_mode. - 2) GET_MODE_BITSIZE (mode) =3D=3D 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 !=3D VOIDmode) { @@ -540,8 +539,6 @@ immed_double_const (HOST_WIDE_INT i0, HO =20 if (GET_MODE_BITSIZE (mode) <=3D HOST_BITS_PER_WIDE_INT) return gen_int_mode (i0, mode); - - gcc_assert (GET_MODE_BITSIZE (mode) =3D=3D 2 * HOST_BITS_PER_WIDE_IN= T); } =20 /* 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 HO= ST_BITS_PER_WIDE_INT bits. So, if you disagree with me, you will necessarily have to quote the definit= ion you are using, explain what the words mean to you _and_ state a specifi= c case in which the code post modification doesn't not conform with the exi= sting 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_WID= E_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_DOUBL= E. > That is, you want to change their spec. We should only do that if we def= ine > 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.