* [PATCH] x86-64: Use TI->SF and TI->DF conversions in soft-fp
@ 2019-01-21 16:16 H.J. Lu
2019-01-21 16:43 ` Uros Bizjak
2019-01-21 23:48 ` Joseph Myers
0 siblings, 2 replies; 12+ messages in thread
From: H.J. Lu @ 2019-01-21 16:16 UTC (permalink / raw)
To: gcc-patches; +Cc: Uros Bizjak
TI->SF and TI->DF conversions in libgcc2.c:
FSTYPE
FUNC (DWtype u)
{
...
}
have no rounding mode support. We should replace __floattisf, __floattidf,
__floatuntisf and __floatuntidf in libgcc2.c with these from soft-fp.
PR libgcc/88931
* config/i386/64/t-softfp-compat (libgcc2-ti-functions): New.
(LIB2FUNCS_EXCLUDE): Likewise.
(libgcc2-ti-softp): Likewise.
(LIB2ADD): Likewise.
---
libgcc/config/i386/64/t-softfp-compat | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/libgcc/config/i386/64/t-softfp-compat b/libgcc/config/i386/64/t-softfp-compat
index 0978695c3a4..abb78032bf5 100644
--- a/libgcc/config/i386/64/t-softfp-compat
+++ b/libgcc/config/i386/64/t-softfp-compat
@@ -13,3 +13,11 @@ libgcc2-tf-functions = _divtc3 _multc3 _powitf2
LIB2FUNCS_EXCLUDE += $(libgcc2-tf-functions)
libgcc2-tf-compats = $(addsuffix .c, $(libgcc2-tf-functions))
LIB2ADD += $(addprefix $(srcdir)/config/i386/64/, $(libgcc2-tf-compats))
+
+# Replace _floatdisf, _floatdidf, _floatundisf and _floatundidf in
+# libgcc2.c, which have no rounding mode support, with floattisf.c,
+# floattidf.c, floatundisf.c and floatundidf.c from soft-fp.
+libgcc2-ti-functions = _floatdisf _floatdidf _floatundisf _floatundidf
+LIB2FUNCS_EXCLUDE += $(libgcc2-ti-functions)
+libgcc2-ti-softp = floattisf.c floattidf.c floatuntisf.c floatuntidf.c
+LIB2ADD += $(addprefix $(srcdir)/soft-fp/, $(libgcc2-ti-softp))
--
2.20.1
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] x86-64: Use TI->SF and TI->DF conversions in soft-fp
2019-01-21 16:16 [PATCH] x86-64: Use TI->SF and TI->DF conversions in soft-fp H.J. Lu
@ 2019-01-21 16:43 ` Uros Bizjak
2019-01-21 16:56 ` H.J. Lu
2019-01-21 23:48 ` Joseph Myers
1 sibling, 1 reply; 12+ messages in thread
From: Uros Bizjak @ 2019-01-21 16:43 UTC (permalink / raw)
To: H.J. Lu; +Cc: gcc-patches
On Mon, Jan 21, 2019 at 5:15 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> TI->SF and TI->DF conversions in libgcc2.c:
>
> FSTYPE
> FUNC (DWtype u)
> {
> ...
> }
>
> have no rounding mode support. We should replace __floattisf, __floattidf,
> __floatuntisf and __floatuntidf in libgcc2.c with these from soft-fp.
>
> PR libgcc/88931
> * config/i386/64/t-softfp-compat (libgcc2-ti-functions): New.
> (LIB2FUNCS_EXCLUDE): Likewise.
> (libgcc2-ti-softp): Likewise.
> (LIB2ADD): Likewise.
> ---
> libgcc/config/i386/64/t-softfp-compat | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/libgcc/config/i386/64/t-softfp-compat b/libgcc/config/i386/64/t-softfp-compat
> index 0978695c3a4..abb78032bf5 100644
> --- a/libgcc/config/i386/64/t-softfp-compat
> +++ b/libgcc/config/i386/64/t-softfp-compat
> @@ -13,3 +13,11 @@ libgcc2-tf-functions = _divtc3 _multc3 _powitf2
> LIB2FUNCS_EXCLUDE += $(libgcc2-tf-functions)
> libgcc2-tf-compats = $(addsuffix .c, $(libgcc2-tf-functions))
> LIB2ADD += $(addprefix $(srcdir)/config/i386/64/, $(libgcc2-tf-compats))
> +
> +# Replace _floatdisf, _floatdidf, _floatundisf and _floatundidf in
> +# libgcc2.c, which have no rounding mode support, with floattisf.c,
> +# floattidf.c, floatundisf.c and floatundidf.c from soft-fp.
> +libgcc2-ti-functions = _floatdisf _floatdidf _floatundisf _floatundidf
> +LIB2FUNCS_EXCLUDE += $(libgcc2-ti-functions)
> +libgcc2-ti-softp = floattisf.c floattidf.c floatuntisf.c floatuntidf.c
> +LIB2ADD += $(addprefix $(srcdir)/soft-fp/, $(libgcc2-ti-softp))
It is not that simple. Please note that libgcc2 functions use FP
instructions in narrower mode (so, in effect still use FPU), while
soft-fp functions don't even touch the FPU, and do everything using
bit twiddling. I think that your change would introduce qoute
noticeable runtime regressions.
Uros.
> --
> 2.20.1
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] x86-64: Use TI->SF and TI->DF conversions in soft-fp
2019-01-21 16:43 ` Uros Bizjak
@ 2019-01-21 16:56 ` H.J. Lu
2019-01-21 16:59 ` Uros Bizjak
0 siblings, 1 reply; 12+ messages in thread
From: H.J. Lu @ 2019-01-21 16:56 UTC (permalink / raw)
To: Uros Bizjak; +Cc: gcc-patches
On Mon, Jan 21, 2019 at 8:43 AM Uros Bizjak <ubizjak@gmail.com> wrote:
>
> On Mon, Jan 21, 2019 at 5:15 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> >
> > TI->SF and TI->DF conversions in libgcc2.c:
> >
> > FSTYPE
> > FUNC (DWtype u)
> > {
> > ...
> > }
> >
> > have no rounding mode support. We should replace __floattisf, __floattidf,
> > __floatuntisf and __floatuntidf in libgcc2.c with these from soft-fp.
> >
> > PR libgcc/88931
> > * config/i386/64/t-softfp-compat (libgcc2-ti-functions): New.
> > (LIB2FUNCS_EXCLUDE): Likewise.
> > (libgcc2-ti-softp): Likewise.
> > (LIB2ADD): Likewise.
> > ---
> > libgcc/config/i386/64/t-softfp-compat | 8 ++++++++
> > 1 file changed, 8 insertions(+)
> >
> > diff --git a/libgcc/config/i386/64/t-softfp-compat b/libgcc/config/i386/64/t-softfp-compat
> > index 0978695c3a4..abb78032bf5 100644
> > --- a/libgcc/config/i386/64/t-softfp-compat
> > +++ b/libgcc/config/i386/64/t-softfp-compat
> > @@ -13,3 +13,11 @@ libgcc2-tf-functions = _divtc3 _multc3 _powitf2
> > LIB2FUNCS_EXCLUDE += $(libgcc2-tf-functions)
> > libgcc2-tf-compats = $(addsuffix .c, $(libgcc2-tf-functions))
> > LIB2ADD += $(addprefix $(srcdir)/config/i386/64/, $(libgcc2-tf-compats))
> > +
> > +# Replace _floatdisf, _floatdidf, _floatundisf and _floatundidf in
> > +# libgcc2.c, which have no rounding mode support, with floattisf.c,
> > +# floattidf.c, floatundisf.c and floatundidf.c from soft-fp.
> > +libgcc2-ti-functions = _floatdisf _floatdidf _floatundisf _floatundidf
> > +LIB2FUNCS_EXCLUDE += $(libgcc2-ti-functions)
> > +libgcc2-ti-softp = floattisf.c floattidf.c floatuntisf.c floatuntidf.c
> > +LIB2ADD += $(addprefix $(srcdir)/soft-fp/, $(libgcc2-ti-softp))
>
> It is not that simple. Please note that libgcc2 functions use FP
> instructions in narrower mode (so, in effect still use FPU), while
> soft-fp functions don't even touch the FPU, and do everything using
> bit twiddling. I think that your change would introduce qoute
> noticeable runtime regressions.
>
By "run-time regressions", did you mean performance or correctness?
--
H.J.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] x86-64: Use TI->SF and TI->DF conversions in soft-fp
2019-01-21 16:56 ` H.J. Lu
@ 2019-01-21 16:59 ` Uros Bizjak
2019-01-21 17:10 ` H.J. Lu
0 siblings, 1 reply; 12+ messages in thread
From: Uros Bizjak @ 2019-01-21 16:59 UTC (permalink / raw)
To: H.J. Lu; +Cc: gcc-patches
On Mon, Jan 21, 2019 at 5:56 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Mon, Jan 21, 2019 at 8:43 AM Uros Bizjak <ubizjak@gmail.com> wrote:
> >
> > On Mon, Jan 21, 2019 at 5:15 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> > >
> > > TI->SF and TI->DF conversions in libgcc2.c:
> > >
> > > FSTYPE
> > > FUNC (DWtype u)
> > > {
> > > ...
> > > }
> > >
> > > have no rounding mode support. We should replace __floattisf, __floattidf,
> > > __floatuntisf and __floatuntidf in libgcc2.c with these from soft-fp.
> > >
> > > PR libgcc/88931
> > > * config/i386/64/t-softfp-compat (libgcc2-ti-functions): New.
> > > (LIB2FUNCS_EXCLUDE): Likewise.
> > > (libgcc2-ti-softp): Likewise.
> > > (LIB2ADD): Likewise.
> > > ---
> > > libgcc/config/i386/64/t-softfp-compat | 8 ++++++++
> > > 1 file changed, 8 insertions(+)
> > >
> > > diff --git a/libgcc/config/i386/64/t-softfp-compat b/libgcc/config/i386/64/t-softfp-compat
> > > index 0978695c3a4..abb78032bf5 100644
> > > --- a/libgcc/config/i386/64/t-softfp-compat
> > > +++ b/libgcc/config/i386/64/t-softfp-compat
> > > @@ -13,3 +13,11 @@ libgcc2-tf-functions = _divtc3 _multc3 _powitf2
> > > LIB2FUNCS_EXCLUDE += $(libgcc2-tf-functions)
> > > libgcc2-tf-compats = $(addsuffix .c, $(libgcc2-tf-functions))
> > > LIB2ADD += $(addprefix $(srcdir)/config/i386/64/, $(libgcc2-tf-compats))
> > > +
> > > +# Replace _floatdisf, _floatdidf, _floatundisf and _floatundidf in
> > > +# libgcc2.c, which have no rounding mode support, with floattisf.c,
> > > +# floattidf.c, floatundisf.c and floatundidf.c from soft-fp.
> > > +libgcc2-ti-functions = _floatdisf _floatdidf _floatundisf _floatundidf
> > > +LIB2FUNCS_EXCLUDE += $(libgcc2-ti-functions)
> > > +libgcc2-ti-softp = floattisf.c floattidf.c floatuntisf.c floatuntidf.c
> > > +LIB2ADD += $(addprefix $(srcdir)/soft-fp/, $(libgcc2-ti-softp))
> >
> > It is not that simple. Please note that libgcc2 functions use FP
> > instructions in narrower mode (so, in effect still use FPU), while
> > soft-fp functions don't even touch the FPU, and do everything using
> > bit twiddling. I think that your change would introduce qoute
> > noticeable runtime regressions.
> >
>
> By "run-time regressions", did you mean performance or correctness?
Performance.
Uros.
> --
> H.J.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] x86-64: Use TI->SF and TI->DF conversions in soft-fp
2019-01-21 16:59 ` Uros Bizjak
@ 2019-01-21 17:10 ` H.J. Lu
2019-01-21 18:37 ` H.J. Lu
0 siblings, 1 reply; 12+ messages in thread
From: H.J. Lu @ 2019-01-21 17:10 UTC (permalink / raw)
To: Uros Bizjak, Wei Xiao, Guo, Xuepeng, Hongtao Liu; +Cc: gcc-patches
On Mon, Jan 21, 2019 at 8:59 AM Uros Bizjak <ubizjak@gmail.com> wrote:
>
> On Mon, Jan 21, 2019 at 5:56 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> >
> > On Mon, Jan 21, 2019 at 8:43 AM Uros Bizjak <ubizjak@gmail.com> wrote:
> > >
> > > On Mon, Jan 21, 2019 at 5:15 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> > > >
> > > > TI->SF and TI->DF conversions in libgcc2.c:
> > > >
> > > > FSTYPE
> > > > FUNC (DWtype u)
> > > > {
> > > > ...
> > > > }
> > > >
> > > > have no rounding mode support. We should replace __floattisf, __floattidf,
> > > > __floatuntisf and __floatuntidf in libgcc2.c with these from soft-fp.
> > > >
> > > > PR libgcc/88931
> > > > * config/i386/64/t-softfp-compat (libgcc2-ti-functions): New.
> > > > (LIB2FUNCS_EXCLUDE): Likewise.
> > > > (libgcc2-ti-softp): Likewise.
> > > > (LIB2ADD): Likewise.
> > > > ---
> > > > libgcc/config/i386/64/t-softfp-compat | 8 ++++++++
> > > > 1 file changed, 8 insertions(+)
> > > >
> > > > diff --git a/libgcc/config/i386/64/t-softfp-compat b/libgcc/config/i386/64/t-softfp-compat
> > > > index 0978695c3a4..abb78032bf5 100644
> > > > --- a/libgcc/config/i386/64/t-softfp-compat
> > > > +++ b/libgcc/config/i386/64/t-softfp-compat
> > > > @@ -13,3 +13,11 @@ libgcc2-tf-functions = _divtc3 _multc3 _powitf2
> > > > LIB2FUNCS_EXCLUDE += $(libgcc2-tf-functions)
> > > > libgcc2-tf-compats = $(addsuffix .c, $(libgcc2-tf-functions))
> > > > LIB2ADD += $(addprefix $(srcdir)/config/i386/64/, $(libgcc2-tf-compats))
> > > > +
> > > > +# Replace _floatdisf, _floatdidf, _floatundisf and _floatundidf in
> > > > +# libgcc2.c, which have no rounding mode support, with floattisf.c,
> > > > +# floattidf.c, floatundisf.c and floatundidf.c from soft-fp.
> > > > +libgcc2-ti-functions = _floatdisf _floatdidf _floatundisf _floatundidf
> > > > +LIB2FUNCS_EXCLUDE += $(libgcc2-ti-functions)
> > > > +libgcc2-ti-softp = floattisf.c floattidf.c floatuntisf.c floatuntidf.c
> > > > +LIB2ADD += $(addprefix $(srcdir)/soft-fp/, $(libgcc2-ti-softp))
> > >
> > > It is not that simple. Please note that libgcc2 functions use FP
> > > instructions in narrower mode (so, in effect still use FPU), while
> > > soft-fp functions don't even touch the FPU, and do everything using
> > > bit twiddling. I think that your change would introduce qoute
> > > noticeable runtime regressions.
> > >
> >
> > By "run-time regressions", did you mean performance or correctness?
>
> Performance.
>
We will check performance change.
--
H.J.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] x86-64: Use TI->SF and TI->DF conversions in soft-fp
2019-01-21 17:10 ` H.J. Lu
@ 2019-01-21 18:37 ` H.J. Lu
0 siblings, 0 replies; 12+ messages in thread
From: H.J. Lu @ 2019-01-21 18:37 UTC (permalink / raw)
To: Uros Bizjak, Wei Xiao, Guo, Xuepeng, Hongtao Liu; +Cc: gcc-patches
On Mon, Jan 21, 2019 at 9:09 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Mon, Jan 21, 2019 at 8:59 AM Uros Bizjak <ubizjak@gmail.com> wrote:
> >
> > On Mon, Jan 21, 2019 at 5:56 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> > >
> > > On Mon, Jan 21, 2019 at 8:43 AM Uros Bizjak <ubizjak@gmail.com> wrote:
> > > >
> > > > On Mon, Jan 21, 2019 at 5:15 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> > > > >
> > > > > TI->SF and TI->DF conversions in libgcc2.c:
> > > > >
> > > > > FSTYPE
> > > > > FUNC (DWtype u)
> > > > > {
> > > > > ...
> > > > > }
> > > > >
> > > > > have no rounding mode support. We should replace __floattisf, __floattidf,
> > > > > __floatuntisf and __floatuntidf in libgcc2.c with these from soft-fp.
> > > > >
> > > > > PR libgcc/88931
> > > > > * config/i386/64/t-softfp-compat (libgcc2-ti-functions): New.
> > > > > (LIB2FUNCS_EXCLUDE): Likewise.
> > > > > (libgcc2-ti-softp): Likewise.
> > > > > (LIB2ADD): Likewise.
> > > > > ---
> > > > > libgcc/config/i386/64/t-softfp-compat | 8 ++++++++
> > > > > 1 file changed, 8 insertions(+)
> > > > >
> > > > > diff --git a/libgcc/config/i386/64/t-softfp-compat b/libgcc/config/i386/64/t-softfp-compat
> > > > > index 0978695c3a4..abb78032bf5 100644
> > > > > --- a/libgcc/config/i386/64/t-softfp-compat
> > > > > +++ b/libgcc/config/i386/64/t-softfp-compat
> > > > > @@ -13,3 +13,11 @@ libgcc2-tf-functions = _divtc3 _multc3 _powitf2
> > > > > LIB2FUNCS_EXCLUDE += $(libgcc2-tf-functions)
> > > > > libgcc2-tf-compats = $(addsuffix .c, $(libgcc2-tf-functions))
> > > > > LIB2ADD += $(addprefix $(srcdir)/config/i386/64/, $(libgcc2-tf-compats))
> > > > > +
> > > > > +# Replace _floatdisf, _floatdidf, _floatundisf and _floatundidf in
> > > > > +# libgcc2.c, which have no rounding mode support, with floattisf.c,
> > > > > +# floattidf.c, floatundisf.c and floatundidf.c from soft-fp.
> > > > > +libgcc2-ti-functions = _floatdisf _floatdidf _floatundisf _floatundidf
> > > > > +LIB2FUNCS_EXCLUDE += $(libgcc2-ti-functions)
> > > > > +libgcc2-ti-softp = floattisf.c floattidf.c floatuntisf.c floatuntidf.c
> > > > > +LIB2ADD += $(addprefix $(srcdir)/soft-fp/, $(libgcc2-ti-softp))
> > > >
> > > > It is not that simple. Please note that libgcc2 functions use FP
> > > > instructions in narrower mode (so, in effect still use FPU), while
> > > > soft-fp functions don't even touch the FPU, and do everything using
> > > > bit twiddling. I think that your change would introduce qoute
> > > > noticeable runtime regressions.
> > > >
> > >
> > > By "run-time regressions", did you mean performance or correctness?
> >
> > Performance.
> >
>
> We will check performance change.
>
I created a micro benchmark on int128/double branch at
https://github.com/hjl-tools/microbenchmark
On Coffee Lake, I got
[hjl@gnu-cfl-2 microbenchmark]$ make
gcc -g -I. -O2 -c -o test.o test.c
gcc -g -c -o libgcc2.o libgcc2.S
gcc -g -c -o soft-fp.o soft-fp.S
gcc -g -c -o floattidf-libgcc2.o floattidf-libgcc2.S
gcc -g -c -o floattidf-soft-fp.o floattidf-soft-fp.S
gcc -o test test.o libgcc2.o soft-fp.o floattidf-libgcc2.o floattidf-soft-fp.o
./test
300000000 loops:
total : 4.4999999767e+16
libgcc2: 2299371252
total : 4.4999999767e+16
soft-fp: 3122038661 (135.78%)
[hjl@gnu-cfl-2 microbenchmark]$
We will collect SPEC CPU 2017 numbers.
--
H.J.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] x86-64: Use TI->SF and TI->DF conversions in soft-fp
2019-01-21 16:16 [PATCH] x86-64: Use TI->SF and TI->DF conversions in soft-fp H.J. Lu
2019-01-21 16:43 ` Uros Bizjak
@ 2019-01-21 23:48 ` Joseph Myers
2019-01-22 0:40 ` Terry Guo
1 sibling, 1 reply; 12+ messages in thread
From: Joseph Myers @ 2019-01-21 23:48 UTC (permalink / raw)
To: H.J. Lu; +Cc: gcc-patches, Uros Bizjak
On Mon, 21 Jan 2019, H.J. Lu wrote:
> TI->SF and TI->DF conversions in libgcc2.c:
>
> FSTYPE
> FUNC (DWtype u)
> {
> ...
> }
>
> have no rounding mode support. We should replace __floattisf, __floattidf,
> __floatuntisf and __floatuntidf in libgcc2.c with these from soft-fp.
Please explain what you mean by "have no rounding mode support" (i.e., the
exact flow through a function that is incorrect in a non-default rounding
mode). This patch is missing testcases - which of course should be
architecture-independent. (Any bug in libgcc2.c should first have an
architecture-independent fix - it can't be considered fixed based on a fix
for one architecture. Then, if some other approach is optimal on
particular architectures, they can get optimized variants.)
I believe all those function implementations are designed so that only a
single rounding occurs, which is for the final result, so no explicit
handling of rounding modes is ever needed (the integer code before then
may set up sticky bits appropriately to ensure the floating-point parts of
the code only need a single rounding, which works in all modes), but maybe
there are bugs in certain cases. To identify the correct fix, we need
details of the exact code path being used (the exact values of the various
macros, choices for the various conditional parts of the function, values
each variable has at each point) and where the existing,
rounding-mode-independent logic goes wrong.
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] x86-64: Use TI->SF and TI->DF conversions in soft-fp
2019-01-21 23:48 ` Joseph Myers
@ 2019-01-22 0:40 ` Terry Guo
2019-01-22 0:58 ` Joseph Myers
0 siblings, 1 reply; 12+ messages in thread
From: Terry Guo @ 2019-01-22 0:40 UTC (permalink / raw)
To: Joseph Myers; +Cc: H.J. Lu, gcc-patches, Uros Bizjak
On Tue, Jan 22, 2019 at 7:48 AM Joseph Myers <joseph@codesourcery.com> wrote:
>
> On Mon, 21 Jan 2019, H.J. Lu wrote:
>
> > TI->SF and TI->DF conversions in libgcc2.c:
> >
> > FSTYPE
> > FUNC (DWtype u)
> > {
> > ...
> > }
> >
> > have no rounding mode support. We should replace __floattisf, __floattidf,
> > __floatuntisf and __floatuntidf in libgcc2.c with these from soft-fp.
>
> Please explain what you mean by "have no rounding mode support" (i.e., the
> exact flow through a function that is incorrect in a non-default rounding
> mode). This patch is missing testcases - which of course should be
> architecture-independent. (Any bug in libgcc2.c should first have an
> architecture-independent fix - it can't be considered fixed based on a fix
> for one architecture. Then, if some other approach is optimal on
> particular architectures, they can get optimized variants.)
>
> I believe all those function implementations are designed so that only a
> single rounding occurs, which is for the final result, so no explicit
> handling of rounding modes is ever needed (the integer code before then
> may set up sticky bits appropriately to ensure the floating-point parts of
> the code only need a single rounding, which works in all modes), but maybe
> there are bugs in certain cases. To identify the correct fix, we need
> details of the exact code path being used (the exact values of the various
> macros, choices for the various conditional parts of the function, values
> each variable has at each point) and where the existing,
> rounding-mode-independent logic goes wrong.
>
> --
> Joseph S. Myers
> joseph@codesourcery.com
Hi Joseph,
I believe HJ is proposing patch to fix bug
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88931. In the test case
of the bug, the "#pragma STDC FENV_ACCESS ON" is used and there are
four rounding modes:
{
ROUNDING (FE_DOWNWARD),
ROUNDING (FE_UPWARD),
ROUNDING (FE_TOWARDZERO),
ROUNDING (FE_TONEAREST)
}
The current _floattisf from libgcc2 doesn't support those four rounding modes.
BR,
Terry
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] x86-64: Use TI->SF and TI->DF conversions in soft-fp
2019-01-22 0:40 ` Terry Guo
@ 2019-01-22 0:58 ` Joseph Myers
2019-01-22 1:34 ` H.J. Lu
0 siblings, 1 reply; 12+ messages in thread
From: Joseph Myers @ 2019-01-22 0:58 UTC (permalink / raw)
To: Terry Guo; +Cc: H.J. Lu, gcc-patches, Uros Bizjak
On Tue, 22 Jan 2019, Terry Guo wrote:
> Hi Joseph,
>
> I believe HJ is proposing patch to fix bug
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88931. In the test case
> of the bug, the "#pragma STDC FENV_ACCESS ON" is used and there are
Which isn't supported by GCC. Any test involving rounding modes should
ensure inputs and results are volatile (or use asm, etc., but volatile is
simpler for tests) to make sure that computations aren't moved across
rounding mode changes (which can happen even with -frounding-math,
-frounding-math only affects a few things like constant folding, without
preventing such code movement).
> The current _floattisf from libgcc2 doesn't support those four rounding modes.
It doesn't need to mention rounding modes anywhere. The basic design of
all those conversion functions is that, given the input, they determine
other inputs to other conversions with the property that only a single
floating-point rounding occurs in the sequence of operations and that the
input to that rounding is similar enough to the input to the original
operation (through careful use of sticky bits etc.) that the result of
that rounding will always be the correct result of the original operation,
independent of the rounding mode.
For example, it's always valid, in any rounding mode, to convert TImode to
SFmode by changing the TImode input to a nicer one (at most top 64 bits
nonzero, say, so that conversions from DImode can be used as an
intermediate step) such that the top 25 bits (starting with the first
nonzero bit, for positive or unsigned arguments) of the two values agree,
and the two values also agree in whether any lower-order bits are nonzero.
That sort of thing is what the code in libgcc2.c is trying to do.
Some of that logic is complex, and it's entirely possible it has bugs.
But the correct fix must be an architecture-independent one in libgcc2.c;
any architecture-specific version is just a subsequent optimization on top
of that. In general, for any bug, you need to work out where the buggy
code is (meaning understanding the intended interfaces between bits of the
compiler that are involved in this question), and fix it there rather than
doing a target-specific workaround. If you want to do a target-specific
workaround (like this patch is), you need to call out up front that your
patch is just a workaround and give strong justification for that approach
(e.g. some way in which the proper general fix would be destabilizing at
the current development stage).
The current description of the bug "Wrong __floattisf and __floattidf are
selected in libgcc" is completely inappropriate unless the assertion is
that one of the #if conditionals in libgcc2.c is wrong (in which case that
#if conditional, or the code it guards, should be corrected).
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] x86-64: Use TI->SF and TI->DF conversions in soft-fp
2019-01-22 0:58 ` Joseph Myers
@ 2019-01-22 1:34 ` H.J. Lu
2019-01-22 2:03 ` Joseph Myers
0 siblings, 1 reply; 12+ messages in thread
From: H.J. Lu @ 2019-01-22 1:34 UTC (permalink / raw)
To: Joseph Myers; +Cc: Terry Guo, gcc-patches, Uros Bizjak
On Mon, Jan 21, 2019 at 4:58 PM Joseph Myers <joseph@codesourcery.com> wrote:
>
> On Tue, 22 Jan 2019, Terry Guo wrote:
>
> > Hi Joseph,
> >
> > I believe HJ is proposing patch to fix bug
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88931. In the test case
> > of the bug, the "#pragma STDC FENV_ACCESS ON" is used and there are
>
> Which isn't supported by GCC. Any test involving rounding modes should
> ensure inputs and results are volatile (or use asm, etc., but volatile is
> simpler for tests) to make sure that computations aren't moved across
> rounding mode changes (which can happen even with -frounding-math,
> -frounding-math only affects a few things like constant folding, without
> preventing such code movement).
>
> > The current _floattisf from libgcc2 doesn't support those four rounding modes.
>
> It doesn't need to mention rounding modes anywhere. The basic design of
> all those conversion functions is that, given the input, they determine
> other inputs to other conversions with the property that only a single
> floating-point rounding occurs in the sequence of operations and that the
> input to that rounding is similar enough to the input to the original
> operation (through careful use of sticky bits etc.) that the result of
> that rounding will always be the correct result of the original operation,
> independent of the rounding mode.
>
> For example, it's always valid, in any rounding mode, to convert TImode to
> SFmode by changing the TImode input to a nicer one (at most top 64 bits
> nonzero, say, so that conversions from DImode can be used as an
> intermediate step) such that the top 25 bits (starting with the first
> nonzero bit, for positive or unsigned arguments) of the two values agree,
> and the two values also agree in whether any lower-order bits are nonzero.
> That sort of thing is what the code in libgcc2.c is trying to do.
>
> Some of that logic is complex, and it's entirely possible it has bugs.
> But the correct fix must be an architecture-independent one in libgcc2.c;
> any architecture-specific version is just a subsequent optimization on top
> of that. In general, for any bug, you need to work out where the buggy
> code is (meaning understanding the intended interfaces between bits of the
> compiler that are involved in this question), and fix it there rather than
> doing a target-specific workaround. If you want to do a target-specific
> workaround (like this patch is), you need to call out up front that your
> patch is just a workaround and give strong justification for that approach
> (e.g. some way in which the proper general fix would be destabilizing at
> the current development stage).
>
> The current description of the bug "Wrong __floattisf and __floattidf are
> selected in libgcc" is completely inappropriate unless the assertion is
> that one of the #if conditionals in libgcc2.c is wrong (in which case that
> #if conditional, or the code it guards, should be corrected).
The testcase at
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88931
with -frounding-math. __floattisf and __floattidf from libgcc2.c give
the wrong results for FE_UPWARD and FE_TOWARDZERO.
--
H.J.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] x86-64: Use TI->SF and TI->DF conversions in soft-fp
2019-01-22 1:34 ` H.J. Lu
@ 2019-01-22 2:03 ` Joseph Myers
2019-01-23 8:19 ` H.J. Lu
0 siblings, 1 reply; 12+ messages in thread
From: Joseph Myers @ 2019-01-22 2:03 UTC (permalink / raw)
To: H.J. Lu; +Cc: Terry Guo, gcc-patches, Uros Bizjak
On Mon, 21 Jan 2019, H.J. Lu wrote:
> The testcase at
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88931
>
> with -frounding-math. __floattisf and __floattidf from libgcc2.c give
> the wrong results for FE_UPWARD and FE_TOWARDZERO.
I suggest writing a test that looks something like
gcc.dg/torture/fp-int-convert-float128-timode-3.c - one that verifies the
conversion of a single value, in a single rounding mode (four such tests
if you want to verify conversions for each of SFmode and DFmode, in each
of two rounding modes). Of course that test wouldn't have any of the dg-*
related to __float128, because this isn't a float128 issue. Given such a
test that uses volatile as appropriate and just does and checks one
conversion, we can be confident we have a real issue (not something about
code movement).
If such a test shows a bug, it will be somewhere in the libgcc2.c code,
with the appropriate fix being in that code. Since that code has lots of
conditionals in it, it would help to identify exactly which cases in that
code are used; that is, which of the
#if FSSIZE >= W_TYPE_SIZE
#elif (LIBGCC2_HAS_DF_MODE && F_MODE_OK (__LIBGCC_DF_MANT_DIG__)) \
|| (LIBGCC2_HAS_XF_MODE && F_MODE_OK (__LIBGCC_XF_MANT_DIG__)) \
|| (LIBGCC2_HAS_TF_MODE && F_MODE_OK (__LIBGCC_TF_MANT_DIG__))
or
#else
cases is being used (and, in the second case, what FSIZE and FTYPE are).
The bug could either be in the code in the selected case, or in the logic
that selects which case to use.
Based on code inspection, it's possible the issue is with
/* No leading bits means u == minimum. */
if (count == 0)
return -(Wtype_MAXp1_F * (Wtype_MAXp1_F / 2));
in the third case (where actually count == 0 only means the high part is
minimum). In which case something like
return Wtype_MAXp1_F * (FSTYPE) (hi | ((UWtype) u != 0));
for the (count == 0) case would address the problem.
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] x86-64: Use TI->SF and TI->DF conversions in soft-fp
2019-01-22 2:03 ` Joseph Myers
@ 2019-01-23 8:19 ` H.J. Lu
0 siblings, 0 replies; 12+ messages in thread
From: H.J. Lu @ 2019-01-23 8:19 UTC (permalink / raw)
To: Joseph Myers; +Cc: Terry Guo, gcc-patches, Uros Bizjak
On Mon, Jan 21, 2019 at 6:03 PM Joseph Myers <joseph@codesourcery.com> wrote:
>
> On Mon, 21 Jan 2019, H.J. Lu wrote:
>
> > The testcase at
> >
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88931
> >
> > with -frounding-math. __floattisf and __floattidf from libgcc2.c give
> > the wrong results for FE_UPWARD and FE_TOWARDZERO.
>
> I suggest writing a test that looks something like
> gcc.dg/torture/fp-int-convert-float128-timode-3.c - one that verifies the
> conversion of a single value, in a single rounding mode (four such tests
> if you want to verify conversions for each of SFmode and DFmode, in each
> of two rounding modes). Of course that test wouldn't have any of the dg-*
> related to __float128, because this isn't a float128 issue. Given such a
> test that uses volatile as appropriate and just does and checks one
> conversion, we can be confident we have a real issue (not something about
> code movement).
>
> If such a test shows a bug, it will be somewhere in the libgcc2.c code,
> with the appropriate fix being in that code. Since that code has lots of
> conditionals in it, it would help to identify exactly which cases in that
> code are used; that is, which of the
>
> #if FSSIZE >= W_TYPE_SIZE
>
> #elif (LIBGCC2_HAS_DF_MODE && F_MODE_OK (__LIBGCC_DF_MANT_DIG__)) \
> || (LIBGCC2_HAS_XF_MODE && F_MODE_OK (__LIBGCC_XF_MANT_DIG__)) \
> || (LIBGCC2_HAS_TF_MODE && F_MODE_OK (__LIBGCC_TF_MANT_DIG__))
>
> or
>
> #else
>
> cases is being used (and, in the second case, what FSIZE and FTYPE are).
> The bug could either be in the code in the selected case, or in the logic
> that selects which case to use.
>
> Based on code inspection, it's possible the issue is with
>
> /* No leading bits means u == minimum. */
> if (count == 0)
> return -(Wtype_MAXp1_F * (Wtype_MAXp1_F / 2));
>
> in the third case (where actually count == 0 only means the high part is
> minimum). In which case something like
>
> return Wtype_MAXp1_F * (FSTYPE) (hi | ((UWtype) u != 0));
>
> for the (count == 0) case would address the problem.
Yes, this works.
--
H.J.
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2019-01-23 2:43 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-21 16:16 [PATCH] x86-64: Use TI->SF and TI->DF conversions in soft-fp H.J. Lu
2019-01-21 16:43 ` Uros Bizjak
2019-01-21 16:56 ` H.J. Lu
2019-01-21 16:59 ` Uros Bizjak
2019-01-21 17:10 ` H.J. Lu
2019-01-21 18:37 ` H.J. Lu
2019-01-21 23:48 ` Joseph Myers
2019-01-22 0:40 ` Terry Guo
2019-01-22 0:58 ` Joseph Myers
2019-01-22 1:34 ` H.J. Lu
2019-01-22 2:03 ` Joseph Myers
2019-01-23 8:19 ` H.J. Lu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).