Re: [Qemu-devel] [PATCH v1 4/8] target-ppc: add vabsdu[b, h, w] instructions

Re: [Qemu-devel] [PATCH v1 4/8] target-ppc: add vabsdu[b, h, w] instructions

[David Gibson]

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

On Fri, Jul 29, 2016 at 09:53:14AM +0530, Richard Henderson wrote:
> <p dir="ltr">On Jul 29, 2016 09:16, David Gibson &lt;david@gibson.dropbear.id.au&gt; wrote:<br>
> &gt; So, I noticed this and was also concerned, but I more or less<br>
> &gt; convinced myself that it would still work, by the magic of 2's<br>
> &gt; complement, as long as sizeof(int) &gt;= 4.</p>
> <p dir="ltr">Consider arguments 0xffffffff, 0.&nbsp; The unsigned absolute difference should be 0xffffffff.&nbsp; With abs(3) we get 1.<br></p>

Ah, good point, I didn't think that through properly.

Btw, your mailer seems to have screwed up and sent this as html only.

-- 
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: 819 bytes --]

Re: [Qemu-devel] [PATCH v1 4/8] target-ppc: add vabsdu[b, h, w] instructions

[Richard Henderson]

On 07/29/2016 10:26 AM, David Gibson wrote:
> Btw, your mailer seems to have screwed up and sent this as html only.

"Sent from my phone" should have been the disclaimer.  ;-)


r~

[Qemu-devel] [PATCH v1 0/8] POWER9 TCG enablements - part2

[Nikunj A Dadhania]

This series contains 11 new instructions for POWER9 described in ISA3.0.

Patches:
  01-02: Changes following instructions:
      divd[u][o][.]: Divide Doubleword Signed/Unsigned
      divw[u][o][.]: Divide Word Signed/Unsigned
  03: dtstsfi[q]    : DFP Test Significance Immediate [Quad]
  04: vabsdub       : Vector Absolute Difference Unsigned Byte
      vabsduh       : Vector Absolute Difference Unsigned Halfword
      vabsduw       : Vector Absolute Difference Unsigned Word
  05: vcmpnezb[.]   : Vector Compare Not Equal or Zero Byte
      vcmpnezh[.]   : Vector Compare Not Equal or Zero Halfword
      vcmpnezw[.]   : Vector Compare Not Equal or Zero Word
  06: vslv          : Vector Shift Left Variable
  07: vsrv          : Vector Shift Right Variable
  08: extswsli      : Extend Sign Word & Shift Left Immediate

Both part1 and part2 pushed here: https://github.com/nikunjad/qemu/tree/p9-tcg

Changelog:
v0:
* Introduce helpers for ISA300 ops
* vabsdu*: drop etype from implementation
* vcmpnez*: collapse the switch case
* vsrv: use reverse traversal to get rid of temporary array
* Include divd/w in this series, as part1 mostly is pushed.

Nikunj A Dadhania (3):
  target-ppc: implement branch-less divw[o][.]
  target-ppc: implement branch-less divd[o][.]
  target-ppc: add extswsli[.] instruction

Sandipan Das (2):
  target-ppc: add dtstsfi[q] instructions
  target-ppc: add vabsdu[b,h,w] instructions

Swapnil Bokade (1):
  target-ppc: add vcmpnez[b,h,w][.] instructions

Vivek Andrew Sha (2):
  target-ppc: add vslv instruction
  target-ppc: add vsrv instruction

 target-ppc/dfp_helper.c         |  35 +++++++++++
 target-ppc/helper.h             |  13 +++++
 target-ppc/int_helper.c         |  89 ++++++++++++++++++++++++++++
 target-ppc/translate.c          | 126 +++++++++++++++++++++++++---------------
 target-ppc/translate/dfp-impl.c |  20 +++++++
 target-ppc/translate/dfp-ops.c  |  14 +++++
 target-ppc/translate/vmx-impl.c |  14 +++++
 target-ppc/translate/vmx-ops.c  |  20 ++++++-
 8 files changed, 281 insertions(+), 50 deletions(-)

-- 
2.7.4

[Qemu-devel] [PATCH v1 4/8] target-ppc: add vabsdu[b, h, w] instructions

[Nikunj A Dadhania]

From: Sandipan Das <sandipandas1990@gmail.com>

Adds following instructions:

vabsdub: Vector Absolute Difference Unsigned Byte
vabsduh: Vector Absolute Difference Unsigned Halfword
vabsduw: Vector Absolute Difference Unsigned Word

Signed-off-by: Sandipan Das <sandipandas1990@gmail.com>
[ use ISA300 define and abs(). Drop etype ]
Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
---
 target-ppc/helper.h             |  3 +++
 target-ppc/int_helper.c         | 22 ++++++++++++++++++++++
 target-ppc/translate/vmx-impl.c |  9 +++++++++
 target-ppc/translate/vmx-ops.c  |  6 +++---
 4 files changed, 37 insertions(+), 3 deletions(-)

diff --git a/target-ppc/helper.h b/target-ppc/helper.h
index 27f2638..1e68060 100644
--- a/target-ppc/helper.h
+++ b/target-ppc/helper.h
@@ -118,6 +118,9 @@ DEF_HELPER_3(vsubudm, void, avr, avr, avr)
 DEF_HELPER_3(vavgub, void, avr, avr, avr)
 DEF_HELPER_3(vavguh, void, avr, avr, avr)
 DEF_HELPER_3(vavguw, void, avr, avr, avr)
+DEF_HELPER_3(vabsdub, void, avr, avr, avr)
+DEF_HELPER_3(vabsduh, void, avr, avr, avr)
+DEF_HELPER_3(vabsduw, void, avr, avr, avr)
 DEF_HELPER_3(vavgsb, void, avr, avr, avr)
 DEF_HELPER_3(vavgsh, void, avr, avr, avr)
 DEF_HELPER_3(vavgsw, void, avr, avr, avr)
diff --git a/target-ppc/int_helper.c b/target-ppc/int_helper.c
index 15947ad..2b375b4 100644
--- a/target-ppc/int_helper.c
+++ b/target-ppc/int_helper.c
@@ -629,6 +629,28 @@ VAVG(w, s32, int64_t, u32, uint64_t)
 #undef VAVG_DO
 #undef VAVG
 
+#define VABSDU_DO(name, element)                                        \
+void helper_v##name(ppc_avr_t *r, ppc_avr_t *a, ppc_avr_t *b)           \
+{                                                                       \
+    int i;                                                              \
+                                                                        \
+    for (i = 0; i < ARRAY_SIZE(r->element); i++) {                      \
+        r->element[i] = abs(a->element[i] - b->element[i]);             \
+    }                                                                   \
+}
+
+/* VABSDU - Vector absolute difference unsigned
+ *   name    - instruction mnemonic suffix (b: byte, h: halfword, w: word)
+ *   element - element type to access from vector
+ */
+#define VABSDU(type, element)                   \
+    VABSDU_DO(absdu##type, element)
+VABSDU(b, u8)
+VABSDU(h, u16)
+VABSDU(w, u32)
+#undef VABSDU_DO
+#undef VABSDU
+
 #define VCF(suffix, cvt, element)                                       \
     void helper_vcf##suffix(CPUPPCState *env, ppc_avr_t *r,             \
                             ppc_avr_t *b, uint32_t uim)                 \
diff --git a/target-ppc/translate/vmx-impl.c b/target-ppc/translate/vmx-impl.c
index a58aa0c..f4ee05b 100644
--- a/target-ppc/translate/vmx-impl.c
+++ b/target-ppc/translate/vmx-impl.c
@@ -285,8 +285,17 @@ GEN_VXFORM(vminsh, 1, 13);
 GEN_VXFORM(vminsw, 1, 14);
 GEN_VXFORM(vminsd, 1, 15);
 GEN_VXFORM(vavgub, 1, 16);
+GEN_VXFORM(vabsdub, 1, 16);
+GEN_VXFORM_DUAL(vavgub, PPC_ALTIVEC, PPC_NONE, \
+                vabsdub, PPC_NONE, PPC2_ISA300)
 GEN_VXFORM(vavguh, 1, 17);
+GEN_VXFORM(vabsduh, 1, 17);
+GEN_VXFORM_DUAL(vavguh, PPC_ALTIVEC, PPC_NONE, \
+                vabsduh, PPC_NONE, PPC2_ISA300)
 GEN_VXFORM(vavguw, 1, 18);
+GEN_VXFORM(vabsduw, 1, 18);
+GEN_VXFORM_DUAL(vavguw, PPC_ALTIVEC, PPC_NONE, \
+                vabsduw, PPC_NONE, PPC2_ISA300)
 GEN_VXFORM(vavgsb, 1, 20);
 GEN_VXFORM(vavgsh, 1, 21);
 GEN_VXFORM(vavgsw, 1, 22);
diff --git a/target-ppc/translate/vmx-ops.c b/target-ppc/translate/vmx-ops.c
index 4d4a62e..abdef6e 100644
--- a/target-ppc/translate/vmx-ops.c
+++ b/target-ppc/translate/vmx-ops.c
@@ -69,9 +69,9 @@ GEN_VXFORM(vminsb, 1, 12),
 GEN_VXFORM(vminsh, 1, 13),
 GEN_VXFORM(vminsw, 1, 14),
 GEN_VXFORM_207(vminsd, 1, 15),
-GEN_VXFORM(vavgub, 1, 16),
-GEN_VXFORM(vavguh, 1, 17),
-GEN_VXFORM(vavguw, 1, 18),
+GEN_VXFORM_DUAL(vavgub, vabsdub, 1, 16, PPC_ALTIVEC, PPC_NONE),
+GEN_VXFORM_DUAL(vavguh, vabsduh, 1, 17, PPC_ALTIVEC, PPC_NONE),
+GEN_VXFORM_DUAL(vavguw, vabsduw, 1, 18, PPC_ALTIVEC, PPC_NONE),
 GEN_VXFORM(vavgsb, 1, 20),
 GEN_VXFORM(vavgsh, 1, 21),
 GEN_VXFORM(vavgsw, 1, 22),
-- 
2.7.4

Re: [Qemu-devel] [PATCH v1 4/8] target-ppc: add vabsdu[b, h, w] instructions

[Richard Henderson]

On 07/28/2016 12:19 PM, Nikunj A Dadhania wrote:
> +        r->element[i] = abs(a->element[i] - b->element[i]);             \
> +    }                                                                   \
> +}
> +
> +/* VABSDU - Vector absolute difference unsigned
> + *   name    - instruction mnemonic suffix (b: byte, h: halfword, w: word)
> + *   element - element type to access from vector
> + */
> +#define VABSDU(type, element)                   \
> +    VABSDU_DO(absdu##type, element)
> +VABSDU(b, u8)
> +VABSDU(h, u16)
> +VABSDU(w, u32)

 From whence are you receiving this abs definition, and how do you expect it to 
work with an unsigned input?

I can only imagine you're getting abs(3), aka int abs(int), from stdlib.h. 
Which technically does work post-arithmetic promotion for u8 and u16, but it 
does not for u32.

I think we'd prefer an explicit (a > b ? a - b : b - a).


r~

Re: [Qemu-devel] [PATCH v1 4/8] target-ppc: add vabsdu[b, h, w] instructions

[Nikunj A Dadhania]

Richard Henderson <rth@twiddle.net> writes:

> On 07/28/2016 12:19 PM, Nikunj A Dadhania wrote:
>> +        r->element[i] = abs(a->element[i] - b->element[i]);             \
>> +    }                                                                   \
>> +}
>> +
>> +/* VABSDU - Vector absolute difference unsigned
>> + *   name    - instruction mnemonic suffix (b: byte, h: halfword, w: word)
>> + *   element - element type to access from vector
>> + */
>> +#define VABSDU(type, element)                   \
>> +    VABSDU_DO(absdu##type, element)
>> +VABSDU(b, u8)
>> +VABSDU(h, u16)
>> +VABSDU(w, u32)
>
> From whence are you receiving this abs definition, and how do you expect it to 
> work with an unsigned input?
>
> I can only imagine you're getting abs(3), aka int abs(int), from stdlib.h. 
> Which technically does work post-arithmetic promotion for u8 and u16, but it 
> does not for u32.

Thanks for pointing that, wasn't aware about that.

> I think we'd prefer an explicit (a > b ? a - b : b - a).

That is what ISA also says, i thought of using library function abs().

Regards
Nikunj

Re: [Qemu-devel] [PATCH v1 4/8] target-ppc: add vabsdu[b, h, w] instructions

[David Gibson]

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

On Thu, Jul 28, 2016 at 06:22:05PM +0530, Richard Henderson wrote:
> On 07/28/2016 12:19 PM, Nikunj A Dadhania wrote:
> > +        r->element[i] = abs(a->element[i] - b->element[i]);             \
> > +    }                                                                   \
> > +}
> > +
> > +/* VABSDU - Vector absolute difference unsigned
> > + *   name    - instruction mnemonic suffix (b: byte, h: halfword, w: word)
> > + *   element - element type to access from vector
> > + */
> > +#define VABSDU(type, element)                   \
> > +    VABSDU_DO(absdu##type, element)
> > +VABSDU(b, u8)
> > +VABSDU(h, u16)
> > +VABSDU(w, u32)
> 
> From whence are you receiving this abs definition, and how do you expect it
> to work with an unsigned input?
> 
> I can only imagine you're getting abs(3), aka int abs(int), from stdlib.h.
> Which technically does work post-arithmetic promotion for u8 and u16, but it
> does not for u32.

So, I noticed this and was also concerned, but I more or less
convinced myself that it would still work, by the magic of 2's
complement, as long as sizeof(int) >= 4.

Maybe I'm wrong, though.

> I think we'd prefer an explicit (a > b ? a - b : b - a).

That probably is easier to follow, though.

-- 
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: 819 bytes --]