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

Message ID 1295465393-1620-1-git-send-email-peter.maydell@linaro.org
State Accepted
Commit 0fad6efce5d3f18278b7239dece3c251b3e7c04d
Headers show

Commit Message

Peter Maydell Jan. 19, 2011, 7:29 p.m. UTC
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(-)

Comments

Schildbach, Wolfgang Jan. 21, 2011, 1:32 p.m. UTC | #1
> -----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. UTC | #2
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. UTC | #3
> 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. UTC | #4
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;
 }