* [Bug tree-optimization/110731] [11/12/13/14 Regression] Wrong-code because of wide-int division since r5-424
2023-07-19 7:18 [Bug tree-optimization/110731] New: [11/12/13/14 Regression] Wrong-code because of wide-int division since r5-424 jakub at gcc dot gnu.org
@ 2023-07-19 7:19 ` jakub at gcc dot gnu.org
2023-07-19 7:39 ` pinskia at gcc dot gnu.org
` (7 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: jakub at gcc dot gnu.org @ 2023-07-19 7:19 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110731
Jakub Jelinek <jakub at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Keywords| |wrong-code
Target Milestone|--- |11.5
Priority|P3 |P2
Ever confirmed|0 |1
Last reconfirmed| |2023-07-19
Status|UNCONFIRMED |NEW
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug tree-optimization/110731] [11/12/13/14 Regression] Wrong-code because of wide-int division since r5-424
2023-07-19 7:18 [Bug tree-optimization/110731] New: [11/12/13/14 Regression] Wrong-code because of wide-int division since r5-424 jakub at gcc dot gnu.org
2023-07-19 7:19 ` [Bug tree-optimization/110731] " jakub at gcc dot gnu.org
@ 2023-07-19 7:39 ` pinskia at gcc dot gnu.org
2023-07-19 7:56 ` jakub at gcc dot gnu.org
` (6 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: pinskia at gcc dot gnu.org @ 2023-07-19 7:39 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110731
--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
This should have done the negative (in the original patch):
1796 /* The quotient is neg if exactly one of the divisor or dividend is
1797 neg. */
1798 if (dividend_neg != divisor_neg)
1799 quotient_len = wi::sub_large (quotient, zeros, 1, quotient,
1800 quotient_len, dividend_prec,
1801 UNSIGNED, 0);
That is you do -((332306998946228968225951765070086143wb + 1)/2)
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug tree-optimization/110731] [11/12/13/14 Regression] Wrong-code because of wide-int division since r5-424
2023-07-19 7:18 [Bug tree-optimization/110731] New: [11/12/13/14 Regression] Wrong-code because of wide-int division since r5-424 jakub at gcc dot gnu.org
2023-07-19 7:19 ` [Bug tree-optimization/110731] " jakub at gcc dot gnu.org
2023-07-19 7:39 ` pinskia at gcc dot gnu.org
@ 2023-07-19 7:56 ` jakub at gcc dot gnu.org
2023-07-19 11:50 ` cvs-commit at gcc dot gnu.org
` (5 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: jakub at gcc dot gnu.org @ 2023-07-19 7:56 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110731
--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
I think the problem is earlier.
If I divide the same values unsigned, while the representation of the wide_int
values
is the same, i.e.
val = {0, -18014398509481984}, len = 2, precision = 119
for x, when divmod_internal_2 is called, in the signed case it sees
x/4wx b_dividend
0x7fffffffc0b0: 0x00000000 0x00000000 0x00000000 0xffc00000
while in the unsigned one
x/4wx b_dividend
0x7fffffffc0b0: 0x00000000 0x00000000 0x00000000 0x00400000
and I think the latter is what should be used in both cases, divmod_internal_2
performs
unsigned division.
So, I think the right fix is:
--- gcc/wide-int.cc.jj 2023-06-12 15:47:22.461502821 +0200
+++ gcc/wide-int.cc 2023-07-19 09:52:40.241661869 +0200
@@ -1911,9 +1911,9 @@ wi::divmod_internal (HOST_WIDE_INT *quot
}
wi_unpack (b_dividend, dividend.get_val (), dividend.get_len (),
- dividend_blocks_needed, dividend_prec, sgn);
+ dividend_blocks_needed, dividend_prec, UNSIGNED);
wi_unpack (b_divisor, divisor.get_val (), divisor.get_len (),
- divisor_blocks_needed, divisor_prec, sgn);
+ divisor_blocks_needed, divisor_prec, UNSIGNED);
m = dividend_blocks_needed;
b_dividend[m] = 0;
because at this point we are after the
/* Make the divisor and dividend positive and remember what we
did. */
if (sgn == SIGNED)
{
if (wi::neg_p (dividend))
{
neg_dividend = -dividend;
dividend = neg_dividend;
dividend_neg = true;
}
if (wi::neg_p (divisor))
{
neg_divisor = -divisor;
divisor = neg_divisor;
divisor_neg = true;
}
}
hunk. Of course, this makes a difference only if either the dividend or
divisor are the smallest negative value of a type.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug tree-optimization/110731] [11/12/13/14 Regression] Wrong-code because of wide-int division since r5-424
2023-07-19 7:18 [Bug tree-optimization/110731] New: [11/12/13/14 Regression] Wrong-code because of wide-int division since r5-424 jakub at gcc dot gnu.org
` (2 preceding siblings ...)
2023-07-19 7:56 ` jakub at gcc dot gnu.org
@ 2023-07-19 11:50 ` cvs-commit at gcc dot gnu.org
2023-07-19 12:24 ` cvs-commit at gcc dot gnu.org
` (4 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2023-07-19 11:50 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110731
--- Comment #3 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:
https://gcc.gnu.org/g:ece799607c841676f4e00c2fea98bbec6976da3f
commit r14-2642-gece799607c841676f4e00c2fea98bbec6976da3f
Author: Jakub Jelinek <jakub@redhat.com>
Date: Wed Jul 19 13:48:53 2023 +0200
wide-int: Fix up wi::divmod_internal [PR110731]
As the following testcase shows, wi::divmod_internal doesn't handle
correctly signed division with precision > 64 when the dividend (and likely
divisor as well) is the type's minimum and the precision isn't divisible
by 64.
A few lines above what the patch hunk changes is:
/* Make the divisor and dividend positive and remember what we
did. */
if (sgn == SIGNED)
{
if (wi::neg_p (dividend))
{
neg_dividend = -dividend;
dividend = neg_dividend;
dividend_neg = true;
}
if (wi::neg_p (divisor))
{
neg_divisor = -divisor;
divisor = neg_divisor;
divisor_neg = true;
}
}
i.e. we negate negative dividend or divisor and remember those.
But, after we do that, when unpacking those values into b_dividend and
b_divisor we need to always treat the wide_ints as UNSIGNED,
because divmod_internal_2 performs an unsigned division only.
Now, if precision <= 64, we don't reach here at all, earlier code
handles it. If dividend or divisor aren't the most negative values,
the negation clears their most significant bit, so it doesn't really
matter if we unpack SIGNED or UNSIGNED. And if precision is multiple
of HOST_BITS_PER_WIDE_INT, there is no difference in behavior, while
-0x80000000000000000000000000000000 negates to
-0x80000000000000000000000000000000 the unpacking of it as SIGNED
or UNSIGNED works the same.
In the testcase, we have signed precision 119 and the dividend is
val = { 0, 0xffc0000000000000 }, len = 2, precision = 119
both before and after negation.
Divisor is
val = { 2 }, len = 1, precision = 119
But we really want to divide 0x400000000000000000000000000000 by 2
unsigned and then negate at the end.
If it is unsigned precision 119 division
0x400000000000000000000000000000 by 2
dividend is
val = { 0, 0xffc0000000000000 }, len = 2, precision = 119
but as we unpack it UNSIGNED, it is unpacked into
0, 0, 0, 0x00400000
The following patch fixes it by always using UNSIGNED unpacking
because we've already negated negative values at that point if
sgn == SIGNED and so most negative constants should be treated as
positive.
2023-07-19 Jakub Jelinek <jakub@redhat.com>
PR tree-optimization/110731
* wide-int.cc (wi::divmod_internal): Always unpack dividend and
divisor as UNSIGNED regardless of sgn.
* gcc.dg/pr110731.c: New test.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug tree-optimization/110731] [11/12/13/14 Regression] Wrong-code because of wide-int division since r5-424
2023-07-19 7:18 [Bug tree-optimization/110731] New: [11/12/13/14 Regression] Wrong-code because of wide-int division since r5-424 jakub at gcc dot gnu.org
` (3 preceding siblings ...)
2023-07-19 11:50 ` cvs-commit at gcc dot gnu.org
@ 2023-07-19 12:24 ` cvs-commit at gcc dot gnu.org
2023-07-19 17:56 ` [Bug tree-optimization/110731] [11/12 " jakub at gcc dot gnu.org
` (3 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2023-07-19 12:24 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110731
--- Comment #4 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-13 branch has been updated by Jakub Jelinek
<jakub@gcc.gnu.org>:
https://gcc.gnu.org/g:f76f411f2cdb62122322e709d503edd2f8e8a530
commit r13-7589-gf76f411f2cdb62122322e709d503edd2f8e8a530
Author: Jakub Jelinek <jakub@redhat.com>
Date: Wed Jul 19 13:48:53 2023 +0200
wide-int: Fix up wi::divmod_internal [PR110731]
As the following testcase shows, wi::divmod_internal doesn't handle
correctly signed division with precision > 64 when the dividend (and likely
divisor as well) is the type's minimum and the precision isn't divisible
by 64.
A few lines above what the patch hunk changes is:
/* Make the divisor and dividend positive and remember what we
did. */
if (sgn == SIGNED)
{
if (wi::neg_p (dividend))
{
neg_dividend = -dividend;
dividend = neg_dividend;
dividend_neg = true;
}
if (wi::neg_p (divisor))
{
neg_divisor = -divisor;
divisor = neg_divisor;
divisor_neg = true;
}
}
i.e. we negate negative dividend or divisor and remember those.
But, after we do that, when unpacking those values into b_dividend and
b_divisor we need to always treat the wide_ints as UNSIGNED,
because divmod_internal_2 performs an unsigned division only.
Now, if precision <= 64, we don't reach here at all, earlier code
handles it. If dividend or divisor aren't the most negative values,
the negation clears their most significant bit, so it doesn't really
matter if we unpack SIGNED or UNSIGNED. And if precision is multiple
of HOST_BITS_PER_WIDE_INT, there is no difference in behavior, while
-0x80000000000000000000000000000000 negates to
-0x80000000000000000000000000000000 the unpacking of it as SIGNED
or UNSIGNED works the same.
In the testcase, we have signed precision 119 and the dividend is
val = { 0, 0xffc0000000000000 }, len = 2, precision = 119
both before and after negation.
Divisor is
val = { 2 }, len = 1, precision = 119
But we really want to divide 0x400000000000000000000000000000 by 2
unsigned and then negate at the end.
If it is unsigned precision 119 division
0x400000000000000000000000000000 by 2
dividend is
val = { 0, 0xffc0000000000000 }, len = 2, precision = 119
but as we unpack it UNSIGNED, it is unpacked into
0, 0, 0, 0x00400000
The following patch fixes it by always using UNSIGNED unpacking
because we've already negated negative values at that point if
sgn == SIGNED and so most negative constants should be treated as
positive.
2023-07-19 Jakub Jelinek <jakub@redhat.com>
PR tree-optimization/110731
* wide-int.cc (wi::divmod_internal): Always unpack dividend and
divisor as UNSIGNED regardless of sgn.
* gcc.dg/pr110731.c: New test.
(cherry picked from commit ece799607c841676f4e00c2fea98bbec6976da3f)
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug tree-optimization/110731] [11/12 Regression] Wrong-code because of wide-int division since r5-424
2023-07-19 7:18 [Bug tree-optimization/110731] New: [11/12/13/14 Regression] Wrong-code because of wide-int division since r5-424 jakub at gcc dot gnu.org
` (4 preceding siblings ...)
2023-07-19 12:24 ` cvs-commit at gcc dot gnu.org
@ 2023-07-19 17:56 ` jakub at gcc dot gnu.org
2023-12-16 0:37 ` cvs-commit at gcc dot gnu.org
` (2 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: jakub at gcc dot gnu.org @ 2023-07-19 17:56 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110731
Jakub Jelinek <jakub at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Summary|[11/12/13/14 Regression] |[11/12 Regression]
|Wrong-code because of |Wrong-code because of
|wide-int division since |wide-int division since
|r5-424 |r5-424
Assignee|unassigned at gcc dot gnu.org |jakub at gcc dot gnu.org
Status|NEW |ASSIGNED
--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed for 13.2+ so far.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug tree-optimization/110731] [11/12 Regression] Wrong-code because of wide-int division since r5-424
2023-07-19 7:18 [Bug tree-optimization/110731] New: [11/12/13/14 Regression] Wrong-code because of wide-int division since r5-424 jakub at gcc dot gnu.org
` (5 preceding siblings ...)
2023-07-19 17:56 ` [Bug tree-optimization/110731] [11/12 " jakub at gcc dot gnu.org
@ 2023-12-16 0:37 ` cvs-commit at gcc dot gnu.org
2023-12-17 13:55 ` cvs-commit at gcc dot gnu.org
2024-06-12 11:58 ` rguenth at gcc dot gnu.org
8 siblings, 0 replies; 10+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2023-12-16 0:37 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110731
--- Comment #6 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-12 branch has been updated by Jakub Jelinek
<jakub@gcc.gnu.org>:
https://gcc.gnu.org/g:9a93c334865af18ce2fd50cd77a9e90939f3c231
commit r12-10046-g9a93c334865af18ce2fd50cd77a9e90939f3c231
Author: Jakub Jelinek <jakub@redhat.com>
Date: Wed Jul 19 13:48:53 2023 +0200
wide-int: Fix up wi::divmod_internal [PR110731]
As the following testcase shows, wi::divmod_internal doesn't handle
correctly signed division with precision > 64 when the dividend (and likely
divisor as well) is the type's minimum and the precision isn't divisible
by 64.
A few lines above what the patch hunk changes is:
/* Make the divisor and dividend positive and remember what we
did. */
if (sgn == SIGNED)
{
if (wi::neg_p (dividend))
{
neg_dividend = -dividend;
dividend = neg_dividend;
dividend_neg = true;
}
if (wi::neg_p (divisor))
{
neg_divisor = -divisor;
divisor = neg_divisor;
divisor_neg = true;
}
}
i.e. we negate negative dividend or divisor and remember those.
But, after we do that, when unpacking those values into b_dividend and
b_divisor we need to always treat the wide_ints as UNSIGNED,
because divmod_internal_2 performs an unsigned division only.
Now, if precision <= 64, we don't reach here at all, earlier code
handles it. If dividend or divisor aren't the most negative values,
the negation clears their most significant bit, so it doesn't really
matter if we unpack SIGNED or UNSIGNED. And if precision is multiple
of HOST_BITS_PER_WIDE_INT, there is no difference in behavior, while
-0x80000000000000000000000000000000 negates to
-0x80000000000000000000000000000000 the unpacking of it as SIGNED
or UNSIGNED works the same.
In the testcase, we have signed precision 119 and the dividend is
val = { 0, 0xffc0000000000000 }, len = 2, precision = 119
both before and after negation.
Divisor is
val = { 2 }, len = 1, precision = 119
But we really want to divide 0x400000000000000000000000000000 by 2
unsigned and then negate at the end.
If it is unsigned precision 119 division
0x400000000000000000000000000000 by 2
dividend is
val = { 0, 0xffc0000000000000 }, len = 2, precision = 119
but as we unpack it UNSIGNED, it is unpacked into
0, 0, 0, 0x00400000
The following patch fixes it by always using UNSIGNED unpacking
because we've already negated negative values at that point if
sgn == SIGNED and so most negative constants should be treated as
positive.
2023-07-19 Jakub Jelinek <jakub@redhat.com>
PR tree-optimization/110731
* wide-int.cc (wi::divmod_internal): Always unpack dividend and
divisor as UNSIGNED regardless of sgn.
* gcc.dg/pr110731.c: New test.
(cherry picked from commit ece799607c841676f4e00c2fea98bbec6976da3f)
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug tree-optimization/110731] [11/12 Regression] Wrong-code because of wide-int division since r5-424
2023-07-19 7:18 [Bug tree-optimization/110731] New: [11/12/13/14 Regression] Wrong-code because of wide-int division since r5-424 jakub at gcc dot gnu.org
` (6 preceding siblings ...)
2023-12-16 0:37 ` cvs-commit at gcc dot gnu.org
@ 2023-12-17 13:55 ` cvs-commit at gcc dot gnu.org
2024-06-12 11:58 ` rguenth at gcc dot gnu.org
8 siblings, 0 replies; 10+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2023-12-17 13:55 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110731
--- Comment #7 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-11 branch has been updated by Jakub Jelinek
<jakub@gcc.gnu.org>:
https://gcc.gnu.org/g:9e30ca09c01124333dc97e8c2cb704b74d3c7b60
commit r11-11148-g9e30ca09c01124333dc97e8c2cb704b74d3c7b60
Author: Jakub Jelinek <jakub@redhat.com>
Date: Wed Jul 19 13:48:53 2023 +0200
wide-int: Fix up wi::divmod_internal [PR110731]
As the following testcase shows, wi::divmod_internal doesn't handle
correctly signed division with precision > 64 when the dividend (and likely
divisor as well) is the type's minimum and the precision isn't divisible
by 64.
A few lines above what the patch hunk changes is:
/* Make the divisor and dividend positive and remember what we
did. */
if (sgn == SIGNED)
{
if (wi::neg_p (dividend))
{
neg_dividend = -dividend;
dividend = neg_dividend;
dividend_neg = true;
}
if (wi::neg_p (divisor))
{
neg_divisor = -divisor;
divisor = neg_divisor;
divisor_neg = true;
}
}
i.e. we negate negative dividend or divisor and remember those.
But, after we do that, when unpacking those values into b_dividend and
b_divisor we need to always treat the wide_ints as UNSIGNED,
because divmod_internal_2 performs an unsigned division only.
Now, if precision <= 64, we don't reach here at all, earlier code
handles it. If dividend or divisor aren't the most negative values,
the negation clears their most significant bit, so it doesn't really
matter if we unpack SIGNED or UNSIGNED. And if precision is multiple
of HOST_BITS_PER_WIDE_INT, there is no difference in behavior, while
-0x80000000000000000000000000000000 negates to
-0x80000000000000000000000000000000 the unpacking of it as SIGNED
or UNSIGNED works the same.
In the testcase, we have signed precision 119 and the dividend is
val = { 0, 0xffc0000000000000 }, len = 2, precision = 119
both before and after negation.
Divisor is
val = { 2 }, len = 1, precision = 119
But we really want to divide 0x400000000000000000000000000000 by 2
unsigned and then negate at the end.
If it is unsigned precision 119 division
0x400000000000000000000000000000 by 2
dividend is
val = { 0, 0xffc0000000000000 }, len = 2, precision = 119
but as we unpack it UNSIGNED, it is unpacked into
0, 0, 0, 0x00400000
The following patch fixes it by always using UNSIGNED unpacking
because we've already negated negative values at that point if
sgn == SIGNED and so most negative constants should be treated as
positive.
2023-07-19 Jakub Jelinek <jakub@redhat.com>
PR tree-optimization/110731
* wide-int.cc (wi::divmod_internal): Always unpack dividend and
divisor as UNSIGNED regardless of sgn.
* gcc.dg/pr110731.c: New test.
(cherry picked from commit ece799607c841676f4e00c2fea98bbec6976da3f)
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug tree-optimization/110731] [11/12 Regression] Wrong-code because of wide-int division since r5-424
2023-07-19 7:18 [Bug tree-optimization/110731] New: [11/12/13/14 Regression] Wrong-code because of wide-int division since r5-424 jakub at gcc dot gnu.org
` (7 preceding siblings ...)
2023-12-17 13:55 ` cvs-commit at gcc dot gnu.org
@ 2024-06-12 11:58 ` rguenth at gcc dot gnu.org
8 siblings, 0 replies; 10+ messages in thread
From: rguenth at gcc dot gnu.org @ 2024-06-12 11:58 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110731
Richard Biener <rguenth at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Known to fail| |11.4.0, 12.3.0
Resolution|--- |FIXED
Status|ASSIGNED |RESOLVED
--- Comment #8 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fixed.
^ permalink raw reply [flat|nested] 10+ messages in thread