This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Fix wide_int unsigned division (PR tree-optimization/69546)
- From: Richard Sandiford <rdsandiford at googlemail dot com>
- To: Jakub Jelinek <jakub at redhat dot com>
- Cc: Mike Stump <mikestump at comcast dot net>, Richard Biener <rguenther at suse dot de>, gcc-patches at gcc dot gnu dot org
- Date: Sat, 30 Jan 2016 12:31:05 +0000
- Subject: Re: [PATCH] Fix wide_int unsigned division (PR tree-optimization/69546)
- Authentication-results: sourceware.org; auth=none
- References: <20160129142014 dot GP3017 at tucnak dot redhat dot com>
Jakub Jelinek <jakub@redhat.com> 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 <jakub@redhat.com>
>
> 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