From patchwork Mon Mar 19 23:31:04 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mike Stump X-Patchwork-Id: 147683 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id CFE89B7004 for ; Tue, 20 Mar 2012 10:31:38 +1100 (EST) Comment: DKIM? See http://www.dkim.org DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=gcc.gnu.org; s=default; x=1332804699; h=Comment: DomainKey-Signature:Received:Received:Received:Received:Received: Subject:Mime-Version:Content-Type:From:In-Reply-To:Date:Cc: Content-Transfer-Encoding:Message-Id:References:To:Mailing-List: Precedence:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:Sender:Delivered-To; bh=KHOT34gLyq5ZQHuQRPpqn9sPzv4=; b=bC40wGRVJi70SOE71RheeaxpDR2Wb0nFc30e3Sjoi4TJf9fp0LkzfsaZCRgk2u YDC0EupQtF68sN01No+APPLZRCUaN3W49wWNNsxv03Ch9PF08eXywsfIvRYxrijK 31osILxUGzL91WIsdr1BnWYZ0UKNff2Cx2N4jRo/qEN6k= Comment: DomainKeys? See http://antispam.yahoo.com/domainkeys DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws; s=default; d=gcc.gnu.org; h=Received:Received:X-SWARE-Spam-Status:X-Spam-Check-By:Received:Received:Received:Subject:Mime-Version:Content-Type:From:In-Reply-To:Date:Cc:Content-Transfer-Encoding:Message-Id:References:To:X-IsSubscribed:Mailing-List:Precedence:List-Id:List-Unsubscribe:List-Archive:List-Post:List-Help:Sender:Delivered-To; b=xO5C/oO34C/6icztTZOni74tM2lFwpA4x9hNqIune2vn1BjfK9uHT+CrB0fKxB fh/XZb3of0f4NQwiDz8BHg7IrlTyYtGmg4DOV7iQ4BeLxxt/KQ70wbrE1pFHy2Bh PGPnConUKboI5ilpv0W0SKvqa9VP9NznwjPtueoLdj3/Q=; 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) From: Mike Stump In-Reply-To: <8762e09sgc.fsf@talisman.home> Date: Mon, 19 Mar 2012 16:31:04 -0700 Cc: gcc-patches Patches 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-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org 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. >> >> 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: 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. 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. */