From patchwork Fri Aug 23 23:23:34 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mike Stump X-Patchwork-Id: 269573 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]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client CN "localhost", Issuer "www.qmailtoaster.com" (not verified)) by ozlabs.org (Postfix) with ESMTPS id 0F8A92C008C for ; Sat, 24 Aug 2013 09:23:48 +1000 (EST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :content-type:mime-version:subject:from:in-reply-to:date:cc :content-transfer-encoding:message-id:references:to; q=dns; s= default; b=hiFuMzI7gPhX5xYAr/Kyv/3iP0ae1wcm/Wix/PfXYMO3B9soZodEH WgOmTEygeYZD7WXNT3NUYU7wUzs64uK5C6pSDUinpYexQIcne7wFXkLS0EUBrD4b Vj9DZSoPILT2959eRghr/c33sWCxdiLDsYxNTHMLNwaEsYO86cbOZQ= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :content-type:mime-version:subject:from:in-reply-to:date:cc :content-transfer-encoding:message-id:references:to; s=default; bh=4zhmTo25bKpinNlPG6MdlS/ZGlw=; b=RgTHQUEj3591uN5vgzDXYXaN8xdm Yk1EST25JQYEzWLQ0fBxALVZql0x4pMEscBkBF1SP6J0AWEc7loJLbgoWmaqvraD gsdWuhjGNjvVdLoAY2FjVmKKDU66UWTUBNejmetwSUDa0gt3FmvfJR+B2ZRTxe6X wl+P1OZEs+QZtdk= Received: (qmail 15862 invoked by alias); 23 Aug 2013 23:23:40 -0000 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 Received: (qmail 15852 invoked by uid 89); 23 Aug 2013 23:23:39 -0000 X-Spam-SWARE-Status: No, score=-4.3 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, KHOP_THREADED, RCVD_IN_DNSWL_NONE, RCVD_IN_HOSTKARMA_NO, RCVD_IN_HOSTKARMA_YE, RP_MATCHES_RCVD, SPF_PASS autolearn=ham version=3.3.2 Received: from qmta04.emeryville.ca.mail.comcast.net (HELO qmta04.emeryville.ca.mail.comcast.net) (76.96.30.40) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Fri, 23 Aug 2013 23:23:38 +0000 Received: from omta19.emeryville.ca.mail.comcast.net ([76.96.30.76]) by qmta04.emeryville.ca.mail.comcast.net with comcast id GP2H1m0021eYJf8A4PPdLC; Fri, 23 Aug 2013 23:23:37 +0000 Received: from up.mrs.kithrup.com ([24.4.193.8]) by omta19.emeryville.ca.mail.comcast.net with comcast id GPPb1m00L0BKwT401PPc5L; Fri, 23 Aug 2013 23:23:36 +0000 Mime-Version: 1.0 (Mac OS X Mail 6.5 \(1508\)) Subject: Re: wide-int branch now up for public comment and review From: Mike Stump In-Reply-To: <87ppt4e9hg.fsf@talisman.default> Date: Fri, 23 Aug 2013 16:23:34 -0700 Cc: Kenneth Zadeck , rguenther@suse.de, gcc-patches , r.sandiford@uk.ibm.com Message-Id: References: <520A9DCC.6080609@naturalbridge.com> <87ppt4e9hg.fsf@talisman.default> To: Richard Sandiford On Aug 23, 2013, at 8:02 AM, Richard Sandiford wrote: >> /* Negate THIS. */ >> inline wide_int_ro >> wide_int_ro::operator - () const >> { >> wide_int_ro r; >> r = wide_int_ro (0) - *this; >> return r; >> } >> >> /* Negate THIS. */ >> inline wide_int_ro >> wide_int_ro::neg () const >> { >> wide_int_ro z = wide_int_ro::from_shwi (0, precision); >> >> gcc_checking_assert (precision); >> return z - *this; >> } > > Why do we need both of these, and why are they implemented slightly > differently? Thanks for the review. neg can go away. There was a time when operator names weren't used, and I added them to make client code more natural. I've removed neg () and updated neg (overflow) to match the style of operator -(). Index: wide-int.cc =================================================================== --- wide-int.cc (revision 201894) +++ wide-int.cc (working copy) @@ -1545,7 +1545,7 @@ wide_int_ro::abs () const gcc_checking_assert (precision); if (sign_mask ()) - result = neg (); + result = -*this; else result = *this; Index: wide-int.h =================================================================== --- wide-int.h (revision 201950) +++ wide-int.h (working copy) @@ -1800,19 +1800,7 @@ class GTY(()) wide_int_ro { /* Negate this. */ inline wide_int_ro operator - () const { - wide_int_ro r; - r = wide_int_ro (0) - *this; - return r; - } - - /* Negate THIS. */ - inline wide_int_ro - neg () const - { - wide_int_ro z = wide_int_ro::from_shwi (0, precision); - - gcc_checking_assert (precision); - return z - *this; + return wide_int_ro (0) - *this; } /* Negate THIS. OVERFLOW is set true if the value cannot be @@ -1820,12 +1808,11 @@ class GTY(()) wide_int_ro { inline wide_int_ro neg (bool *overflow) const { - wide_int_ro z = wide_int_ro::from_shwi (0, precision); - gcc_checking_assert (precision); + *overflow = only_sign_bit_p (); - return z - *this; + return wide_int_ro (0) - *this; } wide_int_ro parity () const; >> template >> inline bool >> fixed_wide_int ::multiple_of_p (const wide_int_ro &factor, >> signop sgn, >> fixed_wide_int *multiple) const >> { >> return wide_int_ro::multiple_of_p (factor, sgn, >> reinterpret_cast (multiple)); >> } > > The patch has several instances of this kind of reinterpret_cast. > It looks like an aliasing violation. The cast is completely unneeded, so I removed it. sdivmod_floor was of the same class, so I removed it as well. These two uses of reinterpret_cast are a bit different from the rest of them. I'll see about addressing the remaining ones in a followup. diff --git a/gcc/wide-int.h b/gcc/wide-int.h index 0a906a9..3fef0d5 100644 --- a/gcc/wide-int.h +++ b/gcc/wide-int.h @@ -3081,9 +3081,7 @@ public: bool multiple_of_p (const wide_int_ro &factor, signop sgn, fixed_wide_int *multiple) const { - return wide_int_ro::multiple_of_p (factor, - sgn, - reinterpret_cast (multiple)); + return wide_int_ro::multiple_of_p (factor, sgn, multiple); } /* Conversion to and from GMP integer representations. */ @@ -3336,7 +3334,7 @@ public: } template inline fixed_wide_int sdivmod_floor (const T &c, fixed_wide_int *mod) const { - return wide_int_ro::sdivmod_floor (c, reinterpret_cast (mod)); + return wide_int_ro::sdivmod_floor (c, mod); } /* Shifting rotating and extracting. */