| Message ID | 20140130164102.GA55091@msticlxl7.ims.intel.com |
|---|---|
| State | New |
| Headers | show |
On Thu, Jan 30, 2014 at 5:41 PM, Ilya Tocar <tocarip.intel@gmail.com> wrote: > This patch removes possible division by zero. > Make check passes. Ok for trunk? > > 2014-01-30 Ilya Tocar <ilya.tocar@intel.com> > > * gcc.target/i386/m512-check.h: Use correct rounding values. > > --- > gcc/testsuite/gcc.target/i386/m512-check.h | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/gcc/testsuite/gcc.target/i386/m512-check.h b/gcc/testsuite/gcc.target/i386/m512-check.h > index 3209039..8441784 100644 > --- a/gcc/testsuite/gcc.target/i386/m512-check.h > +++ b/gcc/testsuite/gcc.target/i386/m512-check.h > @@ -58,7 +58,8 @@ check_rough_##UINON_TYPE (UINON_TYPE u, const VALUE_TYPE *v, \ > \ > for (i = 0; i < ARRAY_SIZE (u.a); i++) \ > { \ > - VALUE_TYPE rel_err = (u.a[i] - v[i]) / v[i]; \ > + VALUE_TYPE rel_err; \ > + rel_err = v[i] != 0 ? (u.a[i] - v[i]) / v[i] : u.a[i]; \ > if (((rel_err < 0) ? -rel_err : rel_err) > eps) \ > { \ > err++; \ We won't get zero from exponential function, so expecting zero result is flawed anyway. If we would like to introduce universal epsilon comparisons into the testsuite, then please read [1]. Being overly pedantic, the definition should be "|(v[i] - u.a[i]) / v[i]|", as stated in [2]. [1] http://randomascii.wordpress.com/2012/02/25/comparing-floating-point-numbers-2012-edition/ [2] http://en.wikipedia.org/wiki/Relative_error Uros.
On 30 Jan 19:24, Uros Bizjak wrote: > On Thu, Jan 30, 2014 at 5:41 PM, Ilya Tocar <tocarip.intel@gmail.com> wrote: > > > This patch removes possible division by zero. > > Make check passes. Ok for trunk? > > > > 2014-01-30 Ilya Tocar <ilya.tocar@intel.com> > > > > * gcc.target/i386/m512-check.h: Use correct rounding values. > > > > --- > > gcc/testsuite/gcc.target/i386/m512-check.h | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/gcc/testsuite/gcc.target/i386/m512-check.h b/gcc/testsuite/gcc.target/i386/m512-check.h > > index 3209039..8441784 100644 > > --- a/gcc/testsuite/gcc.target/i386/m512-check.h > > +++ b/gcc/testsuite/gcc.target/i386/m512-check.h > > @@ -58,7 +58,8 @@ check_rough_##UINON_TYPE (UINON_TYPE u, const VALUE_TYPE *v, \ > > \ > > for (i = 0; i < ARRAY_SIZE (u.a); i++) \ > > { \ > > - VALUE_TYPE rel_err = (u.a[i] - v[i]) / v[i]; \ > > + VALUE_TYPE rel_err; \ > > + rel_err = v[i] != 0 ? (u.a[i] - v[i]) / v[i] : u.a[i]; \ > > if (((rel_err < 0) ? -rel_err : rel_err) > eps) \ > > { \ > > err++; \ > > We won't get zero from exponential function, so expecting zero result > is flawed anyway. > > If we would like to introduce universal epsilon comparisons into the > testsuite, then please read [1]. Being overly pedantic, the definition > should be "|(v[i] - u.a[i]) / v[i]|", as stated in [2]. > > [1] http://randomascii.wordpress.com/2012/02/25/comparing-floating-point-numbers-2012-edition/ > [2] http://en.wikipedia.org/wiki/Relative_error > We get zero from testing zero-masking. Currently we produce 0/0 = NaN. Comparison with NaN is always false, so tests pass. But I think that this should be fixed to avoid division by zero. As for being more pedantic about comparison, I doubt that its useful, when we use 0.0001 as eps.
On Fri, Jan 31, 2014 at 11:00 AM, Ilya Tocar <tocarip.intel@gmail.com> wrote: >> > This patch removes possible division by zero. >> > Make check passes. Ok for trunk? >> > >> > 2014-01-30 Ilya Tocar <ilya.tocar@intel.com> >> > >> > * gcc.target/i386/m512-check.h: Use correct rounding values. >> > >> > --- >> > gcc/testsuite/gcc.target/i386/m512-check.h | 3 ++- >> > 1 file changed, 2 insertions(+), 1 deletion(-) >> > >> > diff --git a/gcc/testsuite/gcc.target/i386/m512-check.h b/gcc/testsuite/gcc.target/i386/m512-check.h >> > index 3209039..8441784 100644 >> > --- a/gcc/testsuite/gcc.target/i386/m512-check.h >> > +++ b/gcc/testsuite/gcc.target/i386/m512-check.h >> > @@ -58,7 +58,8 @@ check_rough_##UINON_TYPE (UINON_TYPE u, const VALUE_TYPE *v, \ >> > \ >> > for (i = 0; i < ARRAY_SIZE (u.a); i++) \ >> > { \ >> > - VALUE_TYPE rel_err = (u.a[i] - v[i]) / v[i]; \ >> > + VALUE_TYPE rel_err; \ >> > + rel_err = v[i] != 0 ? (u.a[i] - v[i]) / v[i] : u.a[i]; \ >> > if (((rel_err < 0) ? -rel_err : rel_err) > eps) \ >> > { \ >> > err++; \ >> >> We won't get zero from exponential function, so expecting zero result >> is flawed anyway. >> >> If we would like to introduce universal epsilon comparisons into the >> testsuite, then please read [1]. Being overly pedantic, the definition >> should be "|(v[i] - u.a[i]) / v[i]|", as stated in [2]. >> >> [1] http://randomascii.wordpress.com/2012/02/25/comparing-floating-point-numbers-2012-edition/ >> [2] http://en.wikipedia.org/wiki/Relative_error >> > > We get zero from testing zero-masking. Currently we produce 0/0 = NaN. > Comparison with NaN is always false, so tests pass. But I think that > this should be fixed to avoid division by zero. As for being more > pedantic about comparison, I doubt that its useful, when we use > 0.0001 as eps. In this case, please add simple check for zero, with the above comment. We don't test exp function, but masking. Uros.
diff --git a/gcc/testsuite/gcc.target/i386/m512-check.h b/gcc/testsuite/gcc.target/i386/m512-check.h index 3209039..8441784 100644 --- a/gcc/testsuite/gcc.target/i386/m512-check.h +++ b/gcc/testsuite/gcc.target/i386/m512-check.h @@ -58,7 +58,8 @@ check_rough_##UINON_TYPE (UINON_TYPE u, const VALUE_TYPE *v, \ \ for (i = 0; i < ARRAY_SIZE (u.a); i++) \ { \ - VALUE_TYPE rel_err = (u.a[i] - v[i]) / v[i]; \ + VALUE_TYPE rel_err; \ + rel_err = v[i] != 0 ? (u.a[i] - v[i]) / v[i] : u.a[i]; \ if (((rel_err < 0) ? -rel_err : rel_err) > eps) \ { \ err++; \