| Message ID | 20181101173852.27257-1-laurent@vivier.eu (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | softfloat: don't execute ppc64 ISA 3.0B instruction if it is not supported | expand |
On 1 November 2018 at 17:38, Laurent Vivier <laurent@vivier.eu> wrote: > commit 27ae5109a2 has introduced an assembly instruction only supported > by ISA 3.0B and it fails to execute on previous versions of the POWER > CPU (like PowerPC G5). > > This patch fixes that by checking the ISA level, and falls back to > the default C function if the instruction is not supported. > > Fixes: 27ae5109a2ba8b6b679cce3e03e16570a34390a0 > (softfloat: Specialize udiv_qrnnd for ppc64) > Signed-off-by: Laurent Vivier <laurent@vivier.eu> > --- > include/fpu/softfloat-macros.h | 39 ++++++++++++++++++++-------------- > 1 file changed, 23 insertions(+), 16 deletions(-) > > diff --git a/include/fpu/softfloat-macros.h b/include/fpu/softfloat-macros.h > index c86687fa5e..fe98b33df9 100644 > --- a/include/fpu/softfloat-macros.h > +++ b/include/fpu/softfloat-macros.h > @@ -78,6 +78,9 @@ this code that are retained. > /* Portions of this work are licensed under the terms of the GNU GPL, > * version 2 or later. See the COPYING file in the top-level directory. > */ > +#if defined(_ARCH_PPC64) > +extern bool have_isa_3_00; > +#endif I was wondering where this bool came from. The answer is that it's defined in tcg/ppc/tcg-target.inc.c. It's ok to use it here because fpu/softfloat.c is only compiled if CONFIG_TCG is true, so the tcg code will be present. The other user of this include file is target/m68k/softfloat.c, which also will only be compiled for a ppc host if CONFIG_TCG. It's a little awkward to be borrowing this tcg/ppc internal flag into the softfloat code, though. thanks -- PMM
On 01/11/2018 18:49, Peter Maydell wrote: > On 1 November 2018 at 17:38, Laurent Vivier <laurent@vivier.eu> wrote: >> commit 27ae5109a2 has introduced an assembly instruction only supported >> by ISA 3.0B and it fails to execute on previous versions of the POWER >> CPU (like PowerPC G5). >> >> This patch fixes that by checking the ISA level, and falls back to >> the default C function if the instruction is not supported. >> >> Fixes: 27ae5109a2ba8b6b679cce3e03e16570a34390a0 >> (softfloat: Specialize udiv_qrnnd for ppc64) >> Signed-off-by: Laurent Vivier <laurent@vivier.eu> >> --- >> include/fpu/softfloat-macros.h | 39 ++++++++++++++++++++-------------- >> 1 file changed, 23 insertions(+), 16 deletions(-) >> >> diff --git a/include/fpu/softfloat-macros.h b/include/fpu/softfloat-macros.h >> index c86687fa5e..fe98b33df9 100644 >> --- a/include/fpu/softfloat-macros.h >> +++ b/include/fpu/softfloat-macros.h >> @@ -78,6 +78,9 @@ this code that are retained. >> /* Portions of this work are licensed under the terms of the GNU GPL, >> * version 2 or later. See the COPYING file in the top-level directory. >> */ >> +#if defined(_ARCH_PPC64) >> +extern bool have_isa_3_00; >> +#endif > > I was wondering where this bool came from. The answer is > that it's defined in tcg/ppc/tcg-target.inc.c. It's ok to > use it here because fpu/softfloat.c is only compiled if > CONFIG_TCG is true, so the tcg code will be present. The > other user of this include file is target/m68k/softfloat.c, > which also will only be compiled for a ppc host if CONFIG_TCG. > > It's a little awkward to be borrowing this tcg/ppc internal > flag into the softfloat code, though. I agree, I was hopping Richard can advice another way to do that. Thanks, Laurent
On 11/1/18 5:38 PM, Laurent Vivier wrote: > commit 27ae5109a2 has introduced an assembly instruction only supported > by ISA 3.0B and it fails to execute on previous versions of the POWER > CPU (like PowerPC G5). > > This patch fixes that by checking the ISA level, and falls back to > the default C function if the instruction is not supported. > > Fixes: 27ae5109a2ba8b6b679cce3e03e16570a34390a0 > (softfloat: Specialize udiv_qrnnd for ppc64) > Signed-off-by: Laurent Vivier <laurent@vivier.eu> > --- > include/fpu/softfloat-macros.h | 39 ++++++++++++++++++++-------------- > 1 file changed, 23 insertions(+), 16 deletions(-) Blah. The divdeu insn was added into v2.06. I knew I had tested this with a power7 host, which I think of as old. But I guess not old enough. I'll work something up. r~
On Thu, Nov 01, 2018 at 06:54:59PM +0100, Laurent Vivier wrote: > On 01/11/2018 18:49, Peter Maydell wrote: > > On 1 November 2018 at 17:38, Laurent Vivier <laurent@vivier.eu> wrote: > >> commit 27ae5109a2 has introduced an assembly instruction only supported > >> by ISA 3.0B and it fails to execute on previous versions of the POWER > >> CPU (like PowerPC G5). > >> > >> This patch fixes that by checking the ISA level, and falls back to > >> the default C function if the instruction is not supported. > >> > >> Fixes: 27ae5109a2ba8b6b679cce3e03e16570a34390a0 > >> (softfloat: Specialize udiv_qrnnd for ppc64) > >> Signed-off-by: Laurent Vivier <laurent@vivier.eu> > >> --- > >> include/fpu/softfloat-macros.h | 39 ++++++++++++++++++++-------------- > >> 1 file changed, 23 insertions(+), 16 deletions(-) > >> > >> diff --git a/include/fpu/softfloat-macros.h b/include/fpu/softfloat-macros.h > >> index c86687fa5e..fe98b33df9 100644 > >> --- a/include/fpu/softfloat-macros.h > >> +++ b/include/fpu/softfloat-macros.h > >> @@ -78,6 +78,9 @@ this code that are retained. > >> /* Portions of this work are licensed under the terms of the GNU GPL, > >> * version 2 or later. See the COPYING file in the top-level directory. > >> */ > >> +#if defined(_ARCH_PPC64) > >> +extern bool have_isa_3_00; > >> +#endif > > > > I was wondering where this bool came from. The answer is > > that it's defined in tcg/ppc/tcg-target.inc.c. It's ok to > > use it here because fpu/softfloat.c is only compiled if > > CONFIG_TCG is true, so the tcg code will be present. The > > other user of this include file is target/m68k/softfloat.c, > > which also will only be compiled for a ppc host if CONFIG_TCG. > > > > It's a little awkward to be borrowing this tcg/ppc internal > > flag into the softfloat code, though. > > I agree, I was hopping Richard can advice another way to do that. We already have ISA version flags in insns_flags & insns_flags2, so I'm hoping we can use those rather than introduce a new bool.
On 03/11/2018 13:57, David Gibson wrote: > On Thu, Nov 01, 2018 at 06:54:59PM +0100, Laurent Vivier wrote: >> On 01/11/2018 18:49, Peter Maydell wrote: >>> On 1 November 2018 at 17:38, Laurent Vivier <laurent@vivier.eu> wrote: >>>> commit 27ae5109a2 has introduced an assembly instruction only supported >>>> by ISA 3.0B and it fails to execute on previous versions of the POWER >>>> CPU (like PowerPC G5). >>>> >>>> This patch fixes that by checking the ISA level, and falls back to >>>> the default C function if the instruction is not supported. >>>> >>>> Fixes: 27ae5109a2ba8b6b679cce3e03e16570a34390a0 >>>> (softfloat: Specialize udiv_qrnnd for ppc64) >>>> Signed-off-by: Laurent Vivier <laurent@vivier.eu> >>>> --- >>>> include/fpu/softfloat-macros.h | 39 ++++++++++++++++++++-------------- >>>> 1 file changed, 23 insertions(+), 16 deletions(-) >>>> >>>> diff --git a/include/fpu/softfloat-macros.h b/include/fpu/softfloat-macros.h >>>> index c86687fa5e..fe98b33df9 100644 >>>> --- a/include/fpu/softfloat-macros.h >>>> +++ b/include/fpu/softfloat-macros.h >>>> @@ -78,6 +78,9 @@ this code that are retained. >>>> /* Portions of this work are licensed under the terms of the GNU GPL, >>>> * version 2 or later. See the COPYING file in the top-level directory. >>>> */ >>>> +#if defined(_ARCH_PPC64) >>>> +extern bool have_isa_3_00; >>>> +#endif >>> >>> I was wondering where this bool came from. The answer is >>> that it's defined in tcg/ppc/tcg-target.inc.c. It's ok to >>> use it here because fpu/softfloat.c is only compiled if >>> CONFIG_TCG is true, so the tcg code will be present. The >>> other user of this include file is target/m68k/softfloat.c, >>> which also will only be compiled for a ppc host if CONFIG_TCG. >>> >>> It's a little awkward to be borrowing this tcg/ppc internal >>> flag into the softfloat code, though. >> >> I agree, I was hopping Richard can advice another way to do that. > > We already have ISA version flags in insns_flags & insns_flags2, so > I'm hoping we can use those rather than introduce a new bool. > Richard has sent another patch using "defined(_ARCH_PWR7)" Thanks, Laurent
diff --git a/include/fpu/softfloat-macros.h b/include/fpu/softfloat-macros.h index c86687fa5e..fe98b33df9 100644 --- a/include/fpu/softfloat-macros.h +++ b/include/fpu/softfloat-macros.h @@ -78,6 +78,9 @@ this code that are retained. /* Portions of this work are licensed under the terms of the GNU GPL, * version 2 or later. See the COPYING file in the top-level directory. */ +#if defined(_ARCH_PPC64) +extern bool have_isa_3_00; +#endif /*---------------------------------------------------------------------------- | Shifts `a' right by the number of bits given in `count'. If any nonzero @@ -647,25 +650,29 @@ static inline uint64_t udiv_qrnnd(uint64_t *r, uint64_t n1, asm("dlgr %0, %1" : "+r"(n) : "r"(d)); *r = n >> 64; return n; -#elif defined(_ARCH_PPC64) - /* From Power ISA 3.0B, programming note for divdeu. */ - uint64_t q1, q2, Q, r1, r2, R; - asm("divdeu %0,%2,%4; divdu %1,%3,%4" - : "=&r"(q1), "=r"(q2) - : "r"(n1), "r"(n0), "r"(d)); - r1 = -(q1 * d); /* low part of (n1<<64) - (q1 * d) */ - r2 = n0 - (q2 * d); - Q = q1 + q2; - R = r1 + r2; - if (R >= d || R < r2) { /* overflow implies R > d */ - Q += 1; - R -= d; - } - *r = R; - return Q; #else uint64_t d0, d1, q0, q1, r1, r0, m; +#if defined(_ARCH_PPC64) + if (have_isa_3_00) { + /* From Power ISA 3.0B, programming note for divdeu. */ + uint64_t q1, q2, Q, r1, r2, R; + asm("divdeu %0,%2,%4; divdu %1,%3,%4" + : "=&r"(q1), "=r"(q2) + : "r"(n1), "r"(n0), "r"(d)); + r1 = -(q1 * d); /* low part of (n1<<64) - (q1 * d) */ + r2 = n0 - (q2 * d); + Q = q1 + q2; + R = r1 + r2; + if (R >= d || R < r2) { /* overflow implies R > d */ + Q += 1; + R -= d; + } + *r = R; + return Q; + } +#endif + d0 = (uint32_t)d; d1 = d >> 32;
commit 27ae5109a2 has introduced an assembly instruction only supported by ISA 3.0B and it fails to execute on previous versions of the POWER CPU (like PowerPC G5). This patch fixes that by checking the ISA level, and falls back to the default C function if the instruction is not supported. Fixes: 27ae5109a2ba8b6b679cce3e03e16570a34390a0 (softfloat: Specialize udiv_qrnnd for ppc64) Signed-off-by: Laurent Vivier <laurent@vivier.eu> --- include/fpu/softfloat-macros.h | 39 ++++++++++++++++++++-------------- 1 file changed, 23 insertions(+), 16 deletions(-)