PR64182: Fix rounding division and modulus
Richard Sandiford
richard.sandiford@arm.com
Fri Dec 12 15:47:00 GMT 2014
Richard Biener <richard.guenther@gmail.com> writes:
> On Thu, Dec 11, 2014 at 1:26 PM, Richard Sandiford
> <richard.sandiford@arm.com> wrote:
>> As pointed out in PR 64182, wide-int rounded division gets the
>> ties-away-from-zero case wrong for odd-numbered dividends, while
>> double_int gets the unsigned case wrong by unconditionally treating
>> a dividend or remainder with the top bit set as negative. As Jakub
>> says, the test used in double_int might also have overflow problems.
>>
>> This patch uses:
>>
>> abs (remainder) >= abs (dividend) - abs (remainder)
>>
>> for both wide-int and double_int and fixes the unsigned case in double_int.
>> I didn't know how to test the double_int change using input code so
>> resorted to doing some double_int arithmetic at the start of main.
>>
>> Thanks to Joseph for the testcase.
>>
>> Tested on x86_64-linux-gnu. OK to install?
>
> Can you add a testcase? You can follow the gcc.dg/plugin/sreal_plugin.c
> example, maybe even make it a generic host_test_plugin.c with separate
> files containing the actual tests.
>
> Otherwise ok.
Ah, hadn't realised we could do that. Much neater than changing main :-)
Here's what I committed after retesting on x86_64-linux-gnu. As well as
the testcase, I changed x - y to the more general wi::sub (x, y).
Thanks,
Richard
gcc/
PR middle-end/64182
* wide-int.h (wi::div_round, wi::mod_round): Fix rounding of tied
cases.
* double-int.c (div_and_round_double): Fix handling of unsigned
cases. Use same rounding approach as wide-int.h.
gcc/testsuite/
2014-xx-xx Richard Sandiford <richard.sandiford@arm.com>
Joseph Myers <joseph@codesourcery.com>
PR middle-end/64182
* gcc.dg/plugin/wide-int-test-1.c,
gcc.dg/plugin/wide-int_plugin.c: New test.
* gcc.dg/plugin/plugin.exp: Register it.
* gnat.dg/round_div.adb: New test.
Index: gcc/wide-int.h
===================================================================
--- gcc/wide-int.h 2014-12-11 14:32:17.708138315 +0000
+++ gcc/wide-int.h 2014-12-11 14:32:17.704138366 +0000
@@ -2616,8 +2616,8 @@ wi::div_round (const T1 &x, const T2 &y,
{
if (sgn == SIGNED)
{
- if (wi::ges_p (wi::abs (remainder),
- wi::lrshift (wi::abs (y), 1)))
+ WI_BINARY_RESULT (T1, T2) abs_remainder = wi::abs (remainder);
+ if (wi::geu_p (abs_remainder, wi::sub (wi::abs (y), abs_remainder)))
{
if (wi::neg_p (x, sgn) != wi::neg_p (y, sgn))
return quotient - 1;
@@ -2627,7 +2627,7 @@ wi::div_round (const T1 &x, const T2 &y,
}
else
{
- if (wi::geu_p (remainder, wi::lrshift (y, 1)))
+ if (wi::geu_p (remainder, wi::sub (y, remainder)))
return quotient + 1;
}
}
@@ -2784,8 +2784,8 @@ wi::mod_round (const T1 &x, const T2 &y,
{
if (sgn == SIGNED)
{
- if (wi::ges_p (wi::abs (remainder),
- wi::lrshift (wi::abs (y), 1)))
+ WI_BINARY_RESULT (T1, T2) abs_remainder = wi::abs (remainder);
+ if (wi::geu_p (abs_remainder, wi::sub (wi::abs (y), abs_remainder)))
{
if (wi::neg_p (x, sgn) != wi::neg_p (y, sgn))
return remainder + y;
@@ -2795,7 +2795,7 @@ wi::mod_round (const T1 &x, const T2 &y,
}
else
{
- if (wi::geu_p (remainder, wi::lrshift (y, 1)))
+ if (wi::geu_p (remainder, wi::sub (y, remainder)))
return remainder - y;
}
}
Index: gcc/double-int.c
===================================================================
--- gcc/double-int.c 2014-12-11 14:32:17.708138315 +0000
+++ gcc/double-int.c 2014-12-11 14:32:17.700138416 +0000
@@ -569,24 +569,23 @@ div_and_round_double (unsigned code, int
{
unsigned HOST_WIDE_INT labs_rem = *lrem;
HOST_WIDE_INT habs_rem = *hrem;
- unsigned HOST_WIDE_INT labs_den = lden, ltwice;
- HOST_WIDE_INT habs_den = hden, htwice;
+ unsigned HOST_WIDE_INT labs_den = lden, lnegabs_rem, ldiff;
+ HOST_WIDE_INT habs_den = hden, hnegabs_rem, hdiff;
/* Get absolute values. */
- if (*hrem < 0)
+ if (!uns && *hrem < 0)
neg_double (*lrem, *hrem, &labs_rem, &habs_rem);
- if (hden < 0)
+ if (!uns && hden < 0)
neg_double (lden, hden, &labs_den, &habs_den);
- /* If (2 * abs (lrem) >= abs (lden)), adjust the quotient. */
- mul_double ((HOST_WIDE_INT) 2, (HOST_WIDE_INT) 0,
- labs_rem, habs_rem, <wice, &htwice);
+ /* If abs(rem) >= abs(den) - abs(rem), adjust the quotient. */
+ neg_double (labs_rem, habs_rem, &lnegabs_rem, &hnegabs_rem);
+ add_double (labs_den, habs_den, lnegabs_rem, hnegabs_rem,
+ &ldiff, &hdiff);
- if (((unsigned HOST_WIDE_INT) habs_den
- < (unsigned HOST_WIDE_INT) htwice)
- || (((unsigned HOST_WIDE_INT) habs_den
- == (unsigned HOST_WIDE_INT) htwice)
- && (labs_den <= ltwice)))
+ if (((unsigned HOST_WIDE_INT) habs_rem
+ > (unsigned HOST_WIDE_INT) hdiff)
+ || (habs_rem == hdiff && labs_rem >= ldiff))
{
if (quo_neg)
/* quo = quo - 1; */
Index: gcc/testsuite/gcc.dg/plugin/wide-int-test-1.c
===================================================================
--- /dev/null 2014-11-19 08:41:51.310561007 +0000
+++ gcc/testsuite/gcc.dg/plugin/wide-int-test-1.c 2014-12-11 14:32:17.704138366 +0000
@@ -0,0 +1,9 @@
+/* Test that pass is inserted and invoked once. */
+/* { dg-do compile } */
+/* { dg-options "-O" } */
+
+int
+main (int argc, char **argv)
+{
+ return 0;
+}
Index: gcc/testsuite/gcc.dg/plugin/wide-int_plugin.c
===================================================================
--- /dev/null 2014-11-19 08:41:51.310561007 +0000
+++ gcc/testsuite/gcc.dg/plugin/wide-int_plugin.c 2014-12-11 14:32:27.792013373 +0000
@@ -0,0 +1,46 @@
+#include "config.h"
+#include "gcc-plugin.h"
+#include "system.h"
+#include "coretypes.h"
+#include "tree.h"
+
+int plugin_is_GPL_compatible;
+
+static void
+test_double_int_round_udiv (void)
+{
+ double_int dmin = { 0, HOST_WIDE_INT_MIN };
+ double_int dmax = { -1, HOST_WIDE_INT_MAX };
+ double_int dnegone = { -1, -1 };
+ double_int mod, div;
+ div = dmin.udivmod (dnegone, ROUND_DIV_EXPR, &mod);
+ if (div.low != 1 || div.high != 0
+ || mod.low != 1 || mod.high != HOST_WIDE_INT_MIN)
+ abort ();
+ div = dmax.udivmod (dnegone, ROUND_DIV_EXPR, &mod);
+ if (div.low != 0 || div.high != 0
+ || mod.low != dmax.low || mod.high != dmax.high)
+ abort ();
+}
+
+static void
+test_wide_int_round_sdiv (void)
+{
+ if (wi::ne_p (wi::div_round (2, 3, SIGNED), 1))
+ abort ();
+ if (wi::ne_p (wi::div_round (1, 3, SIGNED), 0))
+ abort ();
+ if (wi::ne_p (wi::mod_round (2, 3, SIGNED), -1))
+ abort ();
+ if (wi::ne_p (wi::mod_round (1, 3, SIGNED), 1))
+ abort ();
+}
+
+int
+plugin_init (struct plugin_name_args *plugin_info,
+ struct plugin_gcc_version *version)
+{
+ test_double_int_round_udiv ();
+ test_wide_int_round_sdiv ();
+ return 0;
+}
Index: gcc/testsuite/gcc.dg/plugin/plugin.exp
===================================================================
--- gcc/testsuite/gcc.dg/plugin/plugin.exp 2014-12-11 14:32:17.708138315 +0000
+++ gcc/testsuite/gcc.dg/plugin/plugin.exp 2014-12-11 14:32:17.704138366 +0000
@@ -62,6 +62,7 @@ set plugin_test_list [list \
{ sreal_plugin.c sreal-test-1.c } \
{ start_unit_plugin.c start_unit-test-1.c } \
{ finish_unit_plugin.c finish_unit-test-1.c } \
+ { wide-int_plugin.c wide-int-test-1.c } \
]
foreach plugin_test $plugin_test_list {
Index: gcc/testsuite/gnat.dg/round_div.adb
===================================================================
--- /dev/null 2014-11-19 08:41:51.310561007 +0000
+++ gcc/testsuite/gnat.dg/round_div.adb 2014-12-11 14:32:17.700138416 +0000
@@ -0,0 +1,17 @@
+-- { dg-do run }
+-- { dg-options "-O3" }
+procedure Round_Div is
+ type Fixed is delta 1.0 range -2147483648.0 .. 2147483647.0;
+ A : Fixed := 1.0;
+ B : Fixed := 3.0;
+ C : Integer;
+ function Divide (X, Y : Fixed) return Integer is
+ begin
+ return Integer (X / Y);
+ end;
+begin
+ C := Divide (A, B);
+ if C /= 0 then
+ raise Program_Error;
+ end if;
+end Round_Div;
More information about the Gcc-patches
mailing list