Patchwork target-arm: Fix loading of scalar value for Neon multiply-by-scalar

login
register
mail settings
Submitter Peter Maydell
Date Jan. 19, 2011, 7:29 p.m.
Message ID <1295465393-1620-1-git-send-email-peter.maydell@linaro.org>
Download mbox | patch
Permalink /patch/79581/
State New
Headers show

Comments

Peter Maydell - Jan. 19, 2011, 7:29 p.m.
Fix the register and part of register we get the scalar from in
the various "multiply vector by scalar" ops (VMUL by scalar
and friends).

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/translate.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)
Christophe LYON - Jan. 20, 2011, 12:32 p.m.
On 19.01.2011 20:29, Peter Maydell wrote:
> Fix the register and part of register we get the scalar from in
> the various "multiply vector by scalar" ops (VMUL by scalar
> and friends).
> 


I do confirm that with this patch, I can see many improvements in my tests.

Thanks.
Schildbach, Wolfgang - Jan. 21, 2011, 1:32 p.m.
> -----Original Message-----
> From: qemu-devel-bounces+wschi=dolby.com@nongnu.org 
> [mailto:qemu-devel-bounces+wschi=dolby.com@nongnu.org] On 
> Behalf Of Peter Maydell
> Sent: Wednesday, January 19, 2011 11:30 AM
> To: qemu-devel@nongnu.org
> Cc: Christophe Lyon; patches@linaro.org
> Subject: [Qemu-devel] [PATCH] target-arm: Fix loading of 
> scalar value forNeon multiply-by-scalar
> 
> Fix the register and part of register we get the scalar from 
> in the various "multiply vector by scalar" ops (VMUL by 
> scalar and friends).

FWIW, on the two test cases that I have, this patch (together with
Christophe's) does not improve behaviour (see
https://bugs.launchpad.net/bugs/702885).

Regards,
- Wolfgang
Peter Maydell - Jan. 21, 2011, 1:43 p.m.
On 21 January 2011 13:32, Schildbach, Wolfgang <WSCHI@dolby.com> wrote:
> FWIW, on the two test cases that I have, this patch (together with
> Christophe's) does not improve behaviour (see
> https://bugs.launchpad.net/bugs/702885).

Hrm. Can you attached the compiled ARM binaries to that
bug report, to save me the effort of digging out an armcc,
please?

thanks
-- PMM
Schildbach, Wolfgang - Jan. 22, 2011, 2:36 p.m.
> From: qemu-devel-bounces+wschi=dolby.com@nongnu.org 
> [mailto:qemu-devel-bounces+wschi=dolby.com@nongnu.org] On 
> Behalf Of Schildbach, Wolfgang
> 
> > -----Original Message-----
> > From: qemu-devel-bounces+wschi=dolby.com@nongnu.org
> > [mailto:qemu-devel-bounces+wschi=dolby.com@nongnu.org] On Behalf Of 
> > Peter Maydell
> > 
> > Fix the register and part of register we get the scalar from in the 
> > various "multiply vector by scalar" ops (VMUL by scalar and 
> friends).
> 
> FWIW, on the two test cases that I have, this patch (together with
> Christophe's) does not improve behaviour (see 
> https://bugs.launchpad.net/bugs/702885).

Duh. I had missed the greater part of Christophe's patch (I am still
having trouble with my mail client; applying patches off the list is
manual for me).

With both patches applied, indeed the bug filed on launchpad seems
fixed. On my second test case, behaviour is also much improved. Thanks
much!

- Wolfgang
Aurelien Jarno - Jan. 26, 2011, 1:34 p.m.
On Wed, Jan 19, 2011 at 07:29:53PM +0000, Peter Maydell wrote:
> Fix the register and part of register we get the scalar from in
> the various "multiply vector by scalar" ops (VMUL by scalar
> and friends).
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target-arm/translate.c |   12 ++++++------
>  1 files changed, 6 insertions(+), 6 deletions(-)

Thanks, applied.

> diff --git a/target-arm/translate.c b/target-arm/translate.c
> index c60cd18..0c2856a 100644
> --- a/target-arm/translate.c
> +++ b/target-arm/translate.c
> @@ -3608,14 +3608,14 @@ static inline TCGv neon_get_scalar(int size, int reg)
>  {
>      TCGv tmp;
>      if (size == 1) {
> -        tmp = neon_load_reg(reg >> 1, reg & 1);
> -    } else {
> -        tmp = neon_load_reg(reg >> 2, (reg >> 1) & 1);
> -        if (reg & 1) {
> -            gen_neon_dup_low16(tmp);
> -        } else {
> +        tmp = neon_load_reg(reg & 7, reg >> 4);
> +        if (reg & 8) {
>              gen_neon_dup_high16(tmp);
> +        } else {
> +            gen_neon_dup_low16(tmp);
>          }
> +    } else {
> +        tmp = neon_load_reg(reg & 15, reg >> 4);
>      }
>      return tmp;
>  }
> -- 
> 1.6.3.3
> 
> 
>

Patch

diff --git a/target-arm/translate.c b/target-arm/translate.c
index c60cd18..0c2856a 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -3608,14 +3608,14 @@  static inline TCGv neon_get_scalar(int size, int reg)
 {
     TCGv tmp;
     if (size == 1) {
-        tmp = neon_load_reg(reg >> 1, reg & 1);
-    } else {
-        tmp = neon_load_reg(reg >> 2, (reg >> 1) & 1);
-        if (reg & 1) {
-            gen_neon_dup_low16(tmp);
-        } else {
+        tmp = neon_load_reg(reg & 7, reg >> 4);
+        if (reg & 8) {
             gen_neon_dup_high16(tmp);
+        } else {
+            gen_neon_dup_low16(tmp);
         }
+    } else {
+        tmp = neon_load_reg(reg & 15, reg >> 4);
     }
     return tmp;
 }