From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 557 invoked by alias); 21 Mar 2012 17:01:40 -0000 Received: (qmail 531 invoked by uid 22791); 21 Mar 2012 17:01:32 -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 qmta12.westchester.pa.mail.comcast.net (HELO qmta12.westchester.pa.mail.comcast.net) (76.96.59.227) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 21 Mar 2012 17:01:18 +0000 Received: from omta23.westchester.pa.mail.comcast.net ([76.96.62.74]) by qmta12.westchester.pa.mail.comcast.net with comcast id oBYp1i0061c6gX85CH1JHW; Wed, 21 Mar 2012 17:01:18 +0000 Received: from up.mrs.kithrup.com ([24.4.193.8]) by omta23.westchester.pa.mail.comcast.net with comcast id oH1G1i03M0BKwT43jH1HV6; Wed, 21 Mar 2012 17:01:18 +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: Date: Wed, 21 Mar 2012 17:01:00 -0000 Cc: Richard Sandiford , Richard Guenther , gcc-patches Patches Content-Transfer-Encoding: quoted-printable Message-Id: <5F4F06D3-C6F0-47F2-AEB3-1760EC4D2EAE@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> <0A5CBD0C-FC94-4637-B230-1A83372DE91A@comcast.net> <7E568BC6-FD8F-4FD5-8ABF-43FD253D3E8F@comcast.net> To: Michael Matz 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/msg01438.txt.bz2 On Mar 21, 2012, at 6:47 AM, Michael Matz wrote: > Actually, I wouldn't. Ok, thanks for explaining. In light of that, I'd just say, I want to chang= e the spec, the details don't change any for me, only the terminology I mig= ht use. The problem is the spec is causing aborts on valid code, and hence= , either, the code must be duplicated and fixed, or the code has to be fixe= d. I don't see any value in duplicating the code, so, I am left with fixin= g the spec so that valid programs produce valid code. > If the high bit of i1 is set then currently you will get=20 > a negative number produced no matter the absolute value of it. Ok, in the new patch, I'm pushing to change the spec so that the value is s= ign extended and fixing all the code that doesn't conform to that spec. Ri= chard seems to be agreeable with the basic idea, though, we are now sorting= out all the little details to make that happen. If there is any down-side= or details we missed or got wrong, please chime in. > For large negative numbers you'll get a CONST_DOUBLE,=20 > that when interpreted in the large requested mode (which is the only thin= g=20 > that makes sense) is positive. In the new patch, we use sign extension, and when the high bit is set, the = value is interpreted as a negative number is a larger mode. I'll test sign= ed and unsigned constants coming in from above to ensure the right thing ha= ppens. Above the signededness is tracked via the type. In the rtl constan= t, it isn't, so that code will need an assert to prevent large unsigned val= ues from taking this code path. > Hence all values where i1 is between (HWI)1 << (hwibits-1)) and=20 > ((HWI)~0)-1 are the values you're searching for, that show the problem. Presently, I am fixing _all_ problems shown with those values. If you know= of any that we don't address, love to hear about them. > This positive/negative inconsistency doesn't make sense to allow, and the= =20 > assert ensures that it isn't allowed. Don't need the assert when there is no inconsistency, I believe that resolv= ing any inconsistencies should remove the need for the assert. > This would have the seemingly strange effect of disallowing too large=20 > modes only for large values, but that's simply a side-effect of=20 > CONST_DOUBLE and the whole associated machinery not being able to=20 > consistently deal with constants wider than 2*HWI_BITS. I'll move that assert up to the code that has the type information for the = constant. >> if I is 42, we abort. To back the position that spec must not be=20 >> changed, you need to explain at least one thing for which the wrong=20 >> thing will happen if the spec did change. If you want to go down that=20 >> path, you will need to furnish one example where badness happens with 0,= =20 >> not 2, not 3, but 0. >=20 > Huh. Removing the assert wouldn't only allow 0, but also other values.= =20=20 > I don't understand your argumentation: "because for 0 nothing bad happens= ,=20 > that proves that nothing bad happens for any other values which we would= =20 > also allow, hence I can remove the assert"? Right, it merely proves that the assert is wrong and needs fixing. Once yo= u accept that, then we progress on exactly what it should be. This is now = all mostly moot, given that I'm fine with changing the spec as Richard sugg= ested to be a sign-extended constant. Once we have that nice are concrete = definition, the any code conflicts with that, is buggy, and we just fix. S= eems like a nice way forward to me. > It of course doesn't prove anything at all. :-) Only the point I wanted to make; that 0 is safe. As such, it proves t= hat the spec, as you might call it, is wrong. Once that spec is proven wro= ng, trivially, fixing it, isn't unreasonable.