All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] softfloat: don't execute ppc64 ISA 3.0B instruction if it is not supported
@ 2018-11-01 17:38 Laurent Vivier
  2018-11-01 17:49 ` Peter Maydell
  2018-11-01 19:52 ` Richard Henderson
  0 siblings, 2 replies; 6+ messages in thread
From: Laurent Vivier @ 2018-11-01 17:38 UTC (permalink / raw)
  To: qemu-devel
  Cc: Alex Bennée, Aurelien Jarno, Peter Maydell, David Gibson,
	Richard Henderson, Laurent Vivier

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
 
 /*----------------------------------------------------------------------------
 | 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;
 
-- 
2.17.2

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH] softfloat: don't execute ppc64 ISA 3.0B instruction if it is not supported
  2018-11-01 17:38 [Qemu-devel] [PATCH] softfloat: don't execute ppc64 ISA 3.0B instruction if it is not supported Laurent Vivier
@ 2018-11-01 17:49 ` Peter Maydell
  2018-11-01 17:54   ` Laurent Vivier
  2018-11-01 19:52 ` Richard Henderson
  1 sibling, 1 reply; 6+ messages in thread
From: Peter Maydell @ 2018-11-01 17:49 UTC (permalink / raw)
  To: Laurent Vivier
  Cc: QEMU Developers, Alex Bennée, Aurelien Jarno, David Gibson,
	Richard Henderson

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH] softfloat: don't execute ppc64 ISA 3.0B instruction if it is not supported
  2018-11-01 17:49 ` Peter Maydell
@ 2018-11-01 17:54   ` Laurent Vivier
  2018-11-03 12:57     ` David Gibson
  0 siblings, 1 reply; 6+ messages in thread
From: Laurent Vivier @ 2018-11-01 17:54 UTC (permalink / raw)
  To: Peter Maydell
  Cc: QEMU Developers, Alex Bennée, Aurelien Jarno, David Gibson,
	Richard Henderson

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH] softfloat: don't execute ppc64 ISA 3.0B instruction if it is not supported
  2018-11-01 17:38 [Qemu-devel] [PATCH] softfloat: don't execute ppc64 ISA 3.0B instruction if it is not supported Laurent Vivier
  2018-11-01 17:49 ` Peter Maydell
@ 2018-11-01 19:52 ` Richard Henderson
  1 sibling, 0 replies; 6+ messages in thread
From: Richard Henderson @ 2018-11-01 19:52 UTC (permalink / raw)
  To: Laurent Vivier, qemu-devel
  Cc: Alex Bennée, Aurelien Jarno, Peter Maydell, David Gibson

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~

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH] softfloat: don't execute ppc64 ISA 3.0B instruction if it is not supported
  2018-11-01 17:54   ` Laurent Vivier
@ 2018-11-03 12:57     ` David Gibson
  2018-11-04 10:30       ` Laurent Vivier
  0 siblings, 1 reply; 6+ messages in thread
From: David Gibson @ 2018-11-03 12:57 UTC (permalink / raw)
  To: Laurent Vivier
  Cc: Peter Maydell, QEMU Developers, Alex Bennée, Aurelien Jarno,
	Richard Henderson

[-- Attachment #1: Type: text/plain, Size: 2254 bytes --]

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.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH] softfloat: don't execute ppc64 ISA 3.0B instruction if it is not supported
  2018-11-03 12:57     ` David Gibson
@ 2018-11-04 10:30       ` Laurent Vivier
  0 siblings, 0 replies; 6+ messages in thread
From: Laurent Vivier @ 2018-11-04 10:30 UTC (permalink / raw)
  To: David Gibson
  Cc: Peter Maydell, QEMU Developers, Alex Bennée, Aurelien Jarno,
	Richard Henderson

[-- Attachment #1: Type: text/plain, Size: 2196 bytes --]

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2018-11-04 10:30 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-01 17:38 [Qemu-devel] [PATCH] softfloat: don't execute ppc64 ISA 3.0B instruction if it is not supported Laurent Vivier
2018-11-01 17:49 ` Peter Maydell
2018-11-01 17:54   ` Laurent Vivier
2018-11-03 12:57     ` David Gibson
2018-11-04 10:30       ` Laurent Vivier
2018-11-01 19:52 ` Richard Henderson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.