From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 41258 invoked by alias); 30 Jan 2016 12:31:12 -0000 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 Received: (qmail 41244 invoked by uid 89); 30 Jan 2016 12:31:11 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.3 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 spammy=Hx-languages-length:1962, HCc:D*comcast.net X-HELO: mail-wm0-f51.google.com Received: from mail-wm0-f51.google.com (HELO mail-wm0-f51.google.com) (74.125.82.51) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Sat, 30 Jan 2016 12:31:10 +0000 Received: by mail-wm0-f51.google.com with SMTP id r129so13200784wmr.0 for ; Sat, 30 Jan 2016 04:31:09 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:mail-followup-to:cc:subject:references :date:in-reply-to:message-id:user-agent:mime-version:content-type; bh=8a3R8axN4QULQzQXh85r1OElvjq95ZIsGXSlZMr9S0U=; b=Dp6lUQXox7E1amYr+OQET84Q/VHFzN/zXXvwbVcXoBxzfRmRrGXyT1JxvfQSlztzBn r7apj/Kn1ccjbLP2JLdY9w/D/Elpl0IA96+uVugEL8X5yW7FQceggZBKLsKfnSjkYALJ qyaEVxR1eiEzA4StcmGZ6wBB8XInyFgFQYLU5nLwZU1VK0lQgdI4XY2BGiyukbqEvL41 3pNh/D9YiRcqp4PzLs1ESV4rZyXx6qthPxOGom+6obq3ar5Tffz00/jtOW9L48B4+saP 2aGGbdBHOFWq9aVLFrrL8seRPGBE8whmlWz8U+9NkI1EB4g4HwLL/EN3Vx7h/MfBR+AK 2iDw== X-Gm-Message-State: AG10YOSS0mpQy7YQDmiwF9HO5giF5d/Zapsn7r/9zhdzzIz4Hvpwld992whyGTqvDdTzBg== X-Received: by 10.28.105.136 with SMTP id z8mr2521136wmh.71.1454157067293; Sat, 30 Jan 2016 04:31:07 -0800 (PST) Received: from localhost ([95.145.138.245]) by smtp.googlemail.com with ESMTPSA id q75sm2363319wmd.6.2016.01.30.04.31.05 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sat, 30 Jan 2016 04:31:06 -0800 (PST) From: Richard Sandiford To: Jakub Jelinek Mail-Followup-To: Jakub Jelinek ,Mike Stump , Richard Biener , gcc-patches@gcc.gnu.org, rdsandiford@googlemail.com Cc: Mike Stump , Richard Biener , gcc-patches@gcc.gnu.org Subject: Re: [PATCH] Fix wide_int unsigned division (PR tree-optimization/69546) References: <20160129142014.GP3017@tucnak.redhat.com> Date: Sat, 30 Jan 2016 12:31:00 -0000 In-Reply-To: <20160129142014.GP3017@tucnak.redhat.com> (Jakub Jelinek's message of "Fri, 29 Jan 2016 15:20:14 +0100") Message-ID: <87a8nnw8om.fsf@googlemail.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-SW-Source: 2016-01/txt/msg02359.txt.bz2 Jakub Jelinek writes: > As the testcase shows, wide_int unsigned division is broken for > 64bit > precision division of unsigned dividend which have 63rd bit set, and all > higher bits cleared (thus is normalized as 2 HWIs, first with MSB set, > the second 0) and divisor of 1, we return just a single HWI, which is > equivalent to all higher bits set too. If the divisor is > 1, there is > no such problem, as the MSB will not be set after the division. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? > > 2016-01-29 Jakub Jelinek > > PR tree-optimization/69546 > * wide-int.cc (wi::divmod_internal): For unsigned division > where both operands fit into uhwi, if o1 is 1 and o0 has > msb set, if divident_prec is larger than bits per hwi, > clear another quotient word and return 2 instead of 1. > > * gcc.dg/torture/pr69546.c: New test. > > --- gcc/wide-int.cc.jj 2016-01-26 11:46:39.000000000 +0100 > +++ gcc/wide-int.cc 2016-01-29 11:59:33.348852003 +0100 > @@ -1788,15 +1788,25 @@ wi::divmod_internal (HOST_WIDE_INT *quot > { > unsigned HOST_WIDE_INT o0 = dividend.to_uhwi (); > unsigned HOST_WIDE_INT o1 = divisor.to_uhwi (); > + unsigned int quotient_len = 1; > > if (quotient) > - quotient[0] = o0 / o1; > + { > + quotient[0] = o0 / o1; > + if (o1 == 1 > + && (HOST_WIDE_INT) o0 < 0 > + && dividend_prec > HOST_BITS_PER_WIDE_INT) > + { > + quotient[1] = 0; > + quotient_len = 2; > + } > + } > if (remainder) > { > remainder[0] = o0 % o1; > *remainder_len = 1; > } > - return 1; > + return quotient_len; > } Might be wrong, but couldn't the same thing happen for the remainder, e.g. for 0xfffffffe % 0xffffffff ? Maybe we should have a helper function to handle storing uhwis in an array and returning the length. Certainly seems like this function has had its fair share of bugs. Thanks for fixing another. Richard