[Bug middle-end/64182] [5 Regression] wide-int rounding division is broken
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64182 Marek Polacek changed: What|Removed |Added CC||mpolacek at gcc dot gnu.org --- Comment #10 from Marek Polacek --- Fixed?
[Bug middle-end/64182] [5 Regression] wide-int rounding division is broken
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64182 --- Comment #9 from rsandifo at gcc dot gnu.org --- Author: rsandifo Date: Fri Dec 12 15:46:57 2014 New Revision: 218678 URL: https://gcc.gnu.org/viewcvs?rev=218678&root=gcc&view=rev Log: 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 Joseph Myers 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. Added: trunk/gcc/testsuite/gcc.dg/plugin/wide-int-test-1.c trunk/gcc/testsuite/gcc.dg/plugin/wide-int_plugin.c trunk/gcc/testsuite/gnat.dg/round_div.adb Modified: trunk/gcc/ChangeLog trunk/gcc/double-int.c trunk/gcc/testsuite/ChangeLog trunk/gcc/testsuite/gcc.dg/plugin/plugin.exp trunk/gcc/wide-int.h
[Bug middle-end/64182] [5 Regression] wide-int rounding division is broken
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64182 --- Comment #8 from rsandifo at gcc dot gnu.org --- (In reply to Kenneth Zadeck from comment #7) > I do believe that it is possible to test the double int code if you wrote a > program that used fixed point math.I have never used fixed point math > and have no interest in doing so, but double it is the implementation > platform for fixed point math. Yeah. Richi pointed out in the review that we could just use a GCC plugin to test it, so I've got a patch for that going through internal review. Using a plugin is much nicer because it doesn't depend on a non-default language or on fixed-point support. It also means we can test the wi:: routines directly (in addition to the Ada testcase, which I'll still commit).
[Bug middle-end/64182] [5 Regression] wide-int rounding division is broken
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64182 Kenneth Zadeck changed: What|Removed |Added CC||zadeck at naturalbridge dot com --- Comment #7 from Kenneth Zadeck --- I do believe that it is possible to test the double int code if you wrote a program that used fixed point math.I have never used fixed point math and have no interest in doing so, but double it is the implementation platform for fixed point math. kenny
[Bug middle-end/64182] [5 Regression] wide-int rounding division is broken
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64182 --- Comment #6 from rsandifo at gcc dot gnu.org --- (In reply to Jakub Jelinek from comment #5) > Makes sense. Will you also fix double-int.c when you are on this? OK, I'll take the same approach there. I don't know how to test that code "properly", so in the end I just did some double_int arithmetic at the start of main(). Like you say it wasn't rounding correctly in the unsigned case.
[Bug middle-end/64182] [5 Regression] wide-int rounding division is broken
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64182 --- Comment #5 from Jakub Jelinek --- Makes sense. Will you also fix double-int.c when you are on this?
[Bug middle-end/64182] [5 Regression] wide-int rounding division is broken
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64182 --- Comment #4 from rsandifo at gcc dot gnu.org --- (In reply to rsand...@gcc.gnu.org from comment #3) > (In reply to Jakub Jelinek from comment #1) > > Created attachment 34222 [details] > > gcc5-pr64182.patch > > > > So like this (completely untested so far)? I'm hoping that remainder can't > > be ever for signed numbers equal to minimum signed value and thus hopefully > > wi::abs nor lshift of that by 1 should overflow. For UNSIGNED, not sure if > > wi::neg_p () is the right test for whether lshift by 1 will overflow. > > Looks OK to me, but I wonder if we could just use: > > wi::geu_p (y, remainder - y) > > for unsigned, and similarly with abses for signed, which avoids having to > worry about overflow. Will try. Er, of course I mean: wi::geu_p (remainder, y - remainder)
[Bug middle-end/64182] [5 Regression] wide-int rounding division is broken
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64182 rsandifo at gcc dot gnu.org changed: What|Removed |Added Status|UNCONFIRMED |ASSIGNED Last reconfirmed||2014-12-10 Ever confirmed|0 |1 --- Comment #3 from rsandifo at gcc dot gnu.org --- (In reply to Jakub Jelinek from comment #1) > Created attachment 34222 [details] > gcc5-pr64182.patch > > So like this (completely untested so far)? I'm hoping that remainder can't > be ever for signed numbers equal to minimum signed value and thus hopefully > wi::abs nor lshift of that by 1 should overflow. For UNSIGNED, not sure if > wi::neg_p () is the right test for whether lshift by 1 will overflow. Looks OK to me, but I wonder if we could just use: wi::geu_p (y, remainder - y) for unsigned, and similarly with abses for signed, which avoids having to worry about overflow. Will try.
[Bug middle-end/64182] [5 Regression] wide-int rounding division is broken
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64182 --- Comment #2 from Jakub Jelinek --- Note that double-int implementation looks broken too, it doesn't consider uns (negates both anyway if they are "negative"), and it probably doesn't handle the case of too big remainder either.
[Bug middle-end/64182] [5 Regression] wide-int rounding division is broken
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64182 Jakub Jelinek changed: What|Removed |Added CC||jakub at gcc dot gnu.org --- Comment #1 from Jakub Jelinek --- Created attachment 34222 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=34222&action=edit gcc5-pr64182.patch So like this (completely untested so far)? I'm hoping that remainder can't be ever for signed numbers equal to minimum signed value and thus hopefully wi::abs nor lshift of that by 1 should overflow. For UNSIGNED, not sure if wi::neg_p () is the right test for whether lshift by 1 will overflow.
[Bug middle-end/64182] [5 Regression] wide-int rounding division is broken
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64182 Richard Biener changed: What|Removed |Added Priority|P3 |P1 Target Milestone|--- |5.0
