| Message ID | 1296246694-4307-1-git-send-email-sebpop@gmail.com |
|---|---|
| State | New |
| Headers | show |
On Fri, Jan 28, 2011 at 9:31 PM, Sebastian Pop <sebpop@gmail.com> wrote: > Hi Joseph, > > Thanks for your careful review. This patch fixes the remaining > problems. The patch passed regstrap on amd64-linux. Ok for trunk? I don't think adding asserts at this stage (nor including diagnostic-core here in general) is sensible. Can't we do without them? Richard. > Thanks, > Sebastian > > > 2011-01-28 Sebastian Pop <sebastian.pop@amd.com> > Joseph Myers <joseph@codesourcery.com> > > * Makefile.in (hwint.o): Depend on DIAGNOSTIC_CORE_H. > * hwint.c: Include diagnostic-core.h. > (abs_hwi): New. > (gcd): Moved here... > (pos_mul_hwi): New. > (mul_hwi): New. > (least_common_multiple): Moved here... > * hwint.h (gcd): ... from here. > (least_common_multiple): ... from here. > (HOST_WIDE_INT_MIN): New. > (HOST_WIDE_INT_MAX): New. > (abs_hwi): Declared. > (gcd): Declared. > (pos_mul_hwi): Declared. > (mul_hwi): Declared. > (least_common_multiple): Declared. > * omega.c (check_pos_mul): Removed. > (check_mul): Removed. > (omega_solve_geq): Use pos_mul_hwi instead of check_pos_mul and > mul_hwi instead of check_mul. > --- > gcc/ChangeLog | 24 ++++++++++++++++++ > gcc/Makefile.in | 2 +- > gcc/hwint.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ > gcc/hwint.h | 37 ++++++-------------------- > gcc/omega.c | 47 ++++++----------------------------- > 5 files changed, 116 insertions(+), 68 deletions(-) > > diff --git a/gcc/ChangeLog b/gcc/ChangeLog > index f724648..6d130d4 100644 > --- a/gcc/ChangeLog > +++ b/gcc/ChangeLog > @@ -1,3 +1,27 @@ > +2011-01-28 Sebastian Pop <sebastian.pop@amd.com> > + Joseph Myers <joseph@codesourcery.com> > + > + * Makefile.in (hwint.o): Depend on DIAGNOSTIC_CORE_H. > + * hwint.c: Include diagnostic-core.h. > + (abs_hwi): New. > + (gcd): Moved here... > + (pos_mul_hwi): New. > + (mul_hwi): New. > + (least_common_multiple): Moved here... > + * hwint.h (gcd): ... from here. > + (least_common_multiple): ... from here. > + (HOST_WIDE_INT_MIN): New. > + (HOST_WIDE_INT_MAX): New. > + (abs_hwi): Declared. > + (gcd): Declared. > + (pos_mul_hwi): Declared. > + (mul_hwi): Declared. > + (least_common_multiple): Declared. > + * omega.c (check_pos_mul): Removed. > + (check_mul): Removed. > + (omega_solve_geq): Use pos_mul_hwi instead of check_pos_mul and > + mul_hwi instead of check_mul. > + > 2011-01-28 Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE> > > * configure.ac (gcc_cv_ld_static_dynamic): IRIX 6 ld supports > diff --git a/gcc/Makefile.in b/gcc/Makefile.in > index 9a8262a..b2f4bad 100644 > --- a/gcc/Makefile.in > +++ b/gcc/Makefile.in > @@ -2820,7 +2820,7 @@ toplev.o : toplev.c $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H) $(TREE_H) \ > -DTARGET_NAME=\"$(target_noncanonical)\" \ > -c $(srcdir)/toplev.c $(OUTPUT_OPTION) > > -hwint.o : hwint.c $(CONFIG_H) $(SYSTEM_H) > +hwint.o : hwint.c $(CONFIG_H) $(SYSTEM_H) $(DIAGNOSTIC_CORE_H) > > passes.o : passes.c $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H) $(TREE_H) \ > $(RTL_H) $(FUNCTION_H) $(FLAGS_H) xcoffout.h $(INPUT_H) $(INSN_ATTR_H) output.h \ > diff --git a/gcc/hwint.c b/gcc/hwint.c > index 85c1326..a128dc1 100644 > --- a/gcc/hwint.c > +++ b/gcc/hwint.c > @@ -21,6 +21,7 @@ along with GCC; see the file COPYING3. If not see > > #include "config.h" > #include "system.h" > +#include "diagnostic-core.h" > > #if GCC_VERSION < 3004 > > @@ -98,3 +99,76 @@ ffs_hwi (unsigned HOST_WIDE_INT x) > } > > #endif /* GCC_VERSION < 3004 */ > + > +/* Compute the absolute value of X. */ > + > +HOST_WIDE_INT > +abs_hwi (HOST_WIDE_INT x) > +{ > + gcc_checking_assert (x != HOST_WIDE_INT_MIN); > + return x >= 0 ? x : -x; > +} > + > +/* Compute the greatest common divisor of two numbers A and B using > + Euclid's algorithm. */ > + > +HOST_WIDE_INT > +gcd (HOST_WIDE_INT a, HOST_WIDE_INT b) > +{ > + HOST_WIDE_INT x, y, z; > + > + x = abs_hwi (a); > + y = abs_hwi (b); > + > + while (x > 0) > + { > + z = y % x; > + y = x; > + x = z; > + } > + > + return y; > +} > + > +/* For X and Y positive integers, return X multiplied by Y and check > + that the result does not overflow. */ > + > +HOST_WIDE_INT > +pos_mul_hwi (HOST_WIDE_INT x, HOST_WIDE_INT y) > +{ > + if (x != 0) > + gcc_checking_assert ((HOST_WIDE_INT_MAX) / x >= y); > + > + return x * y; > +} > + > +/* Return X multiplied by Y and check that the result does not > + overflow. */ > + > +HOST_WIDE_INT > +mul_hwi (HOST_WIDE_INT x, HOST_WIDE_INT y) > +{ > + gcc_checking_assert (x != HOST_WIDE_INT_MIN > + && y != HOST_WIDE_INT_MIN); > + > + if (x >= 0) > + { > + if (y >= 0) > + return pos_mul_hwi (x, y); > + > + return -pos_mul_hwi (x, -y); > + } > + > + if (y >= 0) > + return -pos_mul_hwi (-x, y); > + > + return pos_mul_hwi (-x, -y); > +} > + > +/* Compute the least common multiple of two numbers A and B . */ > + > +HOST_WIDE_INT > +least_common_multiple (HOST_WIDE_INT a, HOST_WIDE_INT b) > +{ > + return mul_hwi (abs_hwi (a) / gcd (a, b), abs_hwi (b)); > +} > diff --git a/gcc/hwint.h b/gcc/hwint.h > index 1eadd45..fa77b11 100644 > --- a/gcc/hwint.h > +++ b/gcc/hwint.h > @@ -228,33 +228,14 @@ exact_log2 (unsigned HOST_WIDE_INT x) > > #endif /* GCC_VERSION >= 3004 */ > > -/* Compute the greatest common divisor of two numbers using > - Euclid's algorithm. */ > - > -static inline int > -gcd (int a, int b) > -{ > - int x, y, z; > - > - x = abs (a); > - y = abs (b); > - > - while (x > 0) > - { > - z = y % x; > - y = x; > - x = z; > - } > - > - return y; > -} > - > -/* Compute the least common multiple of two numbers A and B . */ > - > -static inline int > -least_common_multiple (int a, int b) > -{ > - return (abs (a) * abs (b) / gcd (a, b)); > -} > +#define HOST_WIDE_INT_MIN (HOST_WIDE_INT) \ > + ((unsigned HOST_WIDE_INT) 1 << (HOST_BITS_PER_WIDE_INT - 1)) > +#define HOST_WIDE_INT_MAX (~(HOST_WIDE_INT_MIN)) > + > +extern HOST_WIDE_INT abs_hwi (HOST_WIDE_INT); > +extern HOST_WIDE_INT gcd (HOST_WIDE_INT, HOST_WIDE_INT); > +extern HOST_WIDE_INT pos_mul_hwi (HOST_WIDE_INT, HOST_WIDE_INT); > +extern HOST_WIDE_INT mul_hwi (HOST_WIDE_INT, HOST_WIDE_INT); > +extern HOST_WIDE_INT least_common_multiple (HOST_WIDE_INT, HOST_WIDE_INT); > > #endif /* ! GCC_HWINT_H */ > diff --git a/gcc/omega.c b/gcc/omega.c > index 1717f8e..c8768d8 100644 > --- a/gcc/omega.c > +++ b/gcc/omega.c > @@ -110,37 +110,6 @@ int_mod (int a, int b) > return a - b * int_div (a, b); > } > > -/* For X and Y positive integers, return X multiplied by Y and check > - that the result does not overflow. */ > - > -static inline int > -check_pos_mul (int x, int y) > -{ > - if (x != 0) > - gcc_assert ((INT_MAX) / x > y); > - > - return x * y; > -} > - > -/* Return X multiplied by Y and check that the result does not > - overflow. */ > - > -static inline int > -check_mul (int x, int y) > -{ > - if (x >= 0) > - { > - if (y >= 0) > - return check_pos_mul (x, y); > - else > - return -check_pos_mul (x, -y); > - } > - else if (y >= 0) > - return -check_pos_mul (-x, y); > - else > - return check_pos_mul (-x, -y); > -} > - > /* Test whether equation E is red. */ > > static inline bool > @@ -3907,8 +3876,8 @@ omega_solve_geq (omega_pb pb, enum omega_result desired_res) > max_splinters += -minC - 1; > else > max_splinters += > - check_pos_mul ((pb->geqs[e].coef[i] - 1), > - (-minC - 1)) / (-minC) + 1; > + pos_mul_hwi ((pb->geqs[e].coef[i] - 1), > + (-minC - 1)) / (-minC) + 1; > } > > /* #ifdef Omega3 */ > @@ -4321,8 +4290,8 @@ omega_solve_geq (omega_pb pb, enum omega_result desired_res) > > for (k = 0; k <= n_vars; k++) > pb->geqs[Ue].coef[k] = > - check_mul (pb->geqs[Ue].coef[k], Lc) + > - check_mul (lbeqn->coef[k], Uc); > + mul_hwi (pb->geqs[Ue].coef[k], Lc) + > + mul_hwi (lbeqn->coef[k], Uc); > > if (dump_file && (dump_flags & TDF_DETAILS)) > { > @@ -4384,8 +4353,8 @@ omega_solve_geq (omega_pb pb, enum omega_result desired_res) > > for (k = n_vars; k >= 0; k--) > pb->geqs[e2].coef[k] = > - check_mul (pb->geqs[Ue].coef[k], Lc) + > - check_mul (pb->geqs[Le].coef[k], Uc); > + mul_hwi (pb->geqs[Ue].coef[k], Lc) + > + mul_hwi (pb->geqs[Le].coef[k], Uc); > > pb->geqs[e2].coef[n_vars + 1] = 0; > pb->geqs[e2].touched = 1; > @@ -4506,8 +4475,8 @@ omega_solve_geq (omega_pb pb, enum omega_result desired_res) > { > for (k = n_vars; k >= 0; k--) > iS->geqs[e2].coef[k] = rS->geqs[e2].coef[k] = > - check_mul (pb->geqs[Ue].coef[k], Lc) + > - check_mul (pb->geqs[Le].coef[k], Uc); > + mul_hwi (pb->geqs[Ue].coef[k], Lc) + > + mul_hwi (pb->geqs[Le].coef[k], Uc); > > iS->geqs[e2].coef[0] -= (Uc - 1) * (Lc - 1); > } > -- > 1.7.1 > >
On Mon, 31 Jan 2011, Richard Guenther wrote: > On Fri, Jan 28, 2011 at 9:31 PM, Sebastian Pop <sebpop@gmail.com> wrote: > > Hi Joseph, > > > > Thanks for your careful review. This patch fixes the remaining > > problems. The patch passed regstrap on amd64-linux. Ok for trunk? > > I don't think adding asserts at this stage (nor including diagnostic-core > here in general) is sensible. Can't we do without them? There are asserts (if not very thorough) in the code being moved from omega.c. The asserts are probably about the safest part of the large series of Graphite patches this is effectively a part of - whether the series as a whole was sensible at this stage is another matter.
On Mon, Jan 31, 2011 at 15:05, Joseph S. Myers <joseph@codesourcery.com> wrote: > On Mon, 31 Jan 2011, Richard Guenther wrote: > >> On Fri, Jan 28, 2011 at 9:31 PM, Sebastian Pop <sebpop@gmail.com> wrote: >> > Hi Joseph, >> > >> > Thanks for your careful review. This patch fixes the remaining >> > problems. The patch passed regstrap on amd64-linux. Ok for trunk? >> >> I don't think adding asserts at this stage (nor including diagnostic-core >> here in general) is sensible. Can't we do without them? > Richard, what do you suggest to get this problem solved? > There are asserts (if not very thorough) in the code being moved from > omega.c. The asserts are probably about the safest part of the large > series of Graphite patches this is effectively a part of - whether the > series as a whole was sensible at this stage is another matter.
On Fri, Feb 4, 2011 at 5:07 AM, Sebastian Pop <sebpop@gmail.com> wrote: > On Mon, Jan 31, 2011 at 15:05, Joseph S. Myers <joseph@codesourcery.com> wrote: >> On Mon, 31 Jan 2011, Richard Guenther wrote: >> >>> On Fri, Jan 28, 2011 at 9:31 PM, Sebastian Pop <sebpop@gmail.com> wrote: >>> > Hi Joseph, >>> > >>> > Thanks for your careful review. This patch fixes the remaining >>> > problems. The patch passed regstrap on amd64-linux. Ok for trunk? >>> >>> I don't think adding asserts at this stage (nor including diagnostic-core >>> here in general) is sensible. Can't we do without them? >> > > Richard, what do you suggest to get this problem solved? I'd like to postpone this patch to stage1. The implementations were just copied from one file to another with the issues Joseph mentions pre-existing. There is no existing bugreport that shows they are a problem, but changing them, especially adding asserts, may expose some. Thus, please queue the patch for stage1 and ping it there again. Richard. >> There are asserts (if not very thorough) in the code being moved from >> omega.c. The asserts are probably about the safest part of the large >> series of Graphite patches this is effectively a part of - whether the >> series as a whole was sensible at this stage is another matter. >
diff --git a/gcc/ChangeLog b/gcc/ChangeLog index f724648..6d130d4 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,27 @@ +2011-01-28 Sebastian Pop <sebastian.pop@amd.com> + Joseph Myers <joseph@codesourcery.com> + + * Makefile.in (hwint.o): Depend on DIAGNOSTIC_CORE_H. + * hwint.c: Include diagnostic-core.h. + (abs_hwi): New. + (gcd): Moved here... + (pos_mul_hwi): New. + (mul_hwi): New. + (least_common_multiple): Moved here... + * hwint.h (gcd): ... from here. + (least_common_multiple): ... from here. + (HOST_WIDE_INT_MIN): New. + (HOST_WIDE_INT_MAX): New. + (abs_hwi): Declared. + (gcd): Declared. + (pos_mul_hwi): Declared. + (mul_hwi): Declared. + (least_common_multiple): Declared. + * omega.c (check_pos_mul): Removed. + (check_mul): Removed. + (omega_solve_geq): Use pos_mul_hwi instead of check_pos_mul and + mul_hwi instead of check_mul. + 2011-01-28 Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE> * configure.ac (gcc_cv_ld_static_dynamic): IRIX 6 ld supports diff --git a/gcc/Makefile.in b/gcc/Makefile.in index 9a8262a..b2f4bad 100644 --- a/gcc/Makefile.in +++ b/gcc/Makefile.in @@ -2820,7 +2820,7 @@ toplev.o : toplev.c $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H) $(TREE_H) \ -DTARGET_NAME=\"$(target_noncanonical)\" \ -c $(srcdir)/toplev.c $(OUTPUT_OPTION) -hwint.o : hwint.c $(CONFIG_H) $(SYSTEM_H) +hwint.o : hwint.c $(CONFIG_H) $(SYSTEM_H) $(DIAGNOSTIC_CORE_H) passes.o : passes.c $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H) $(TREE_H) \ $(RTL_H) $(FUNCTION_H) $(FLAGS_H) xcoffout.h $(INPUT_H) $(INSN_ATTR_H) output.h \ diff --git a/gcc/hwint.c b/gcc/hwint.c index 85c1326..a128dc1 100644 --- a/gcc/hwint.c +++ b/gcc/hwint.c @@ -21,6 +21,7 @@ along with GCC; see the file COPYING3. If not see #include "config.h" #include "system.h" +#include "diagnostic-core.h" #if GCC_VERSION < 3004 @@ -98,3 +99,76 @@ ffs_hwi (unsigned HOST_WIDE_INT x) } #endif /* GCC_VERSION < 3004 */ + +/* Compute the absolute value of X. */ + +HOST_WIDE_INT +abs_hwi (HOST_WIDE_INT x) +{ + gcc_checking_assert (x != HOST_WIDE_INT_MIN); + return x >= 0 ? x : -x; +} + +/* Compute the greatest common divisor of two numbers A and B using + Euclid's algorithm. */ + +HOST_WIDE_INT +gcd (HOST_WIDE_INT a, HOST_WIDE_INT b) +{ + HOST_WIDE_INT x, y, z; + + x = abs_hwi (a); + y = abs_hwi (b); + + while (x > 0) + { + z = y % x; + y = x; + x = z; + } + + return y; +} + +/* For X and Y positive integers, return X multiplied by Y and check + that the result does not overflow. */ + +HOST_WIDE_INT +pos_mul_hwi (HOST_WIDE_INT x, HOST_WIDE_INT y) +{ + if (x != 0) + gcc_checking_assert ((HOST_WIDE_INT_MAX) / x >= y); + + return x * y; +} + +/* Return X multiplied by Y and check that the result does not + overflow. */ + +HOST_WIDE_INT +mul_hwi (HOST_WIDE_INT x, HOST_WIDE_INT y) +{ + gcc_checking_assert (x != HOST_WIDE_INT_MIN + && y != HOST_WIDE_INT_MIN); + + if (x >= 0) + { + if (y >= 0) + return pos_mul_hwi (x, y); + + return -pos_mul_hwi (x, -y); + } + + if (y >= 0) + return -pos_mul_hwi (-x, y); + + return pos_mul_hwi (-x, -y); +} + +/* Compute the least common multiple of two numbers A and B . */ + +HOST_WIDE_INT +least_common_multiple (HOST_WIDE_INT a, HOST_WIDE_INT b) +{ + return mul_hwi (abs_hwi (a) / gcd (a, b), abs_hwi (b)); +} diff --git a/gcc/hwint.h b/gcc/hwint.h index 1eadd45..fa77b11 100644 --- a/gcc/hwint.h +++ b/gcc/hwint.h @@ -228,33 +228,14 @@ exact_log2 (unsigned HOST_WIDE_INT x) #endif /* GCC_VERSION >= 3004 */ -/* Compute the greatest common divisor of two numbers using - Euclid's algorithm. */ - -static inline int -gcd (int a, int b) -{ - int x, y, z; - - x = abs (a); - y = abs (b); - - while (x > 0) - { - z = y % x; - y = x; - x = z; - } - - return y; -} - -/* Compute the least common multiple of two numbers A and B . */ - -static inline int -least_common_multiple (int a, int b) -{ - return (abs (a) * abs (b) / gcd (a, b)); -} +#define HOST_WIDE_INT_MIN (HOST_WIDE_INT) \ + ((unsigned HOST_WIDE_INT) 1 << (HOST_BITS_PER_WIDE_INT - 1)) +#define HOST_WIDE_INT_MAX (~(HOST_WIDE_INT_MIN)) + +extern HOST_WIDE_INT abs_hwi (HOST_WIDE_INT); +extern HOST_WIDE_INT gcd (HOST_WIDE_INT, HOST_WIDE_INT); +extern HOST_WIDE_INT pos_mul_hwi (HOST_WIDE_INT, HOST_WIDE_INT); +extern HOST_WIDE_INT mul_hwi (HOST_WIDE_INT, HOST_WIDE_INT); +extern HOST_WIDE_INT least_common_multiple (HOST_WIDE_INT, HOST_WIDE_INT); #endif /* ! GCC_HWINT_H */ diff --git a/gcc/omega.c b/gcc/omega.c index 1717f8e..c8768d8 100644 --- a/gcc/omega.c +++ b/gcc/omega.c @@ -110,37 +110,6 @@ int_mod (int a, int b) return a - b * int_div (a, b); } -/* For X and Y positive integers, return X multiplied by Y and check - that the result does not overflow. */ - -static inline int -check_pos_mul (int x, int y) -{ - if (x != 0) - gcc_assert ((INT_MAX) / x > y); - - return x * y; -} - -/* Return X multiplied by Y and check that the result does not - overflow. */ - -static inline int -check_mul (int x, int y) -{ - if (x >= 0) - { - if (y >= 0) - return check_pos_mul (x, y); - else - return -check_pos_mul (x, -y); - } - else if (y >= 0) - return -check_pos_mul (-x, y); - else - return check_pos_mul (-x, -y); -} - /* Test whether equation E is red. */ static inline bool @@ -3907,8 +3876,8 @@ omega_solve_geq (omega_pb pb, enum omega_result desired_res) max_splinters += -minC - 1; else max_splinters += - check_pos_mul ((pb->geqs[e].coef[i] - 1), - (-minC - 1)) / (-minC) + 1; + pos_mul_hwi ((pb->geqs[e].coef[i] - 1), + (-minC - 1)) / (-minC) + 1; } /* #ifdef Omega3 */ @@ -4321,8 +4290,8 @@ omega_solve_geq (omega_pb pb, enum omega_result desired_res) for (k = 0; k <= n_vars; k++) pb->geqs[Ue].coef[k] = - check_mul (pb->geqs[Ue].coef[k], Lc) + - check_mul (lbeqn->coef[k], Uc); + mul_hwi (pb->geqs[Ue].coef[k], Lc) + + mul_hwi (lbeqn->coef[k], Uc); if (dump_file && (dump_flags & TDF_DETAILS)) { @@ -4384,8 +4353,8 @@ omega_solve_geq (omega_pb pb, enum omega_result desired_res) for (k = n_vars; k >= 0; k--) pb->geqs[e2].coef[k] = - check_mul (pb->geqs[Ue].coef[k], Lc) + - check_mul (pb->geqs[Le].coef[k], Uc); + mul_hwi (pb->geqs[Ue].coef[k], Lc) + + mul_hwi (pb->geqs[Le].coef[k], Uc); pb->geqs[e2].coef[n_vars + 1] = 0; pb->geqs[e2].touched = 1; @@ -4506,8 +4475,8 @@ omega_solve_geq (omega_pb pb, enum omega_result desired_res) { for (k = n_vars; k >= 0; k--) iS->geqs[e2].coef[k] = rS->geqs[e2].coef[k] = - check_mul (pb->geqs[Ue].coef[k], Lc) + - check_mul (pb->geqs[Le].coef[k], Uc); + mul_hwi (pb->geqs[Ue].coef[k], Lc) + + mul_hwi (pb->geqs[Le].coef[k], Uc); iS->geqs[e2].coef[0] -= (Uc - 1) * (Lc - 1); }