diff mbox series

[1/1] tcg: Fix typos in helper_gvec_sar{8, 32, 64}v

Message ID 20190613041619.32102-2-richard.henderson@linaro.org
State Accepted
Commit 899f08ad1d1231dbbfa67298413f05ed2679fb02
Headers show
Series tcg: queued patch | expand

Commit Message

Richard Henderson June 13, 2019, 4:16 a.m. UTC 
The loop is written with scalars, not vectors.
Use the correct type when incrementing.

Fixes: 5ee5c14cacd
Reported-by: Laurent Vivier <lvivier@redhat.com>
Tested-by: Laurent Vivier <lvivier@redhat.com>

Reviewed-by: Laurent Vivier <lvivier@redhat.com>

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

---
 accel/tcg/tcg-runtime-gvec.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

-- 
2.17.1

Comments

Aleksandar Markovic June 14, 2019, 5:22 a.m. UTC | #1 
On Jun 13, 2019 6:16 AM, "Richard Henderson" <richard.henderson@linaro.org>
wrote:
>

> The loop is written with scalars, not vectors.

> Use the correct type when incrementing.

>

> Fixes: 5ee5c14cacd

> Reported-by: Laurent Vivier <lvivier@redhat.com>

> Tested-by: Laurent Vivier <lvivier@redhat.com>

> Reviewed-by: Laurent Vivier <lvivier@redhat.com>

> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

> ---


This is certainly not a case of “typo” (which would mean that you
accidentally typed “vec8” instead od “int8_t”). So, change the title to
“tcg: Fix loop step in
helper_gvec_sar{8, 32, 64}v” or similar.

>  accel/tcg/tcg-runtime-gvec.c | 6 +++---

>  1 file changed, 3 insertions(+), 3 deletions(-)

>

> diff --git a/accel/tcg/tcg-runtime-gvec.c b/accel/tcg/tcg-runtime-gvec.c

> index 3b6052fe97..51cb29ca79 100644

> --- a/accel/tcg/tcg-runtime-gvec.c

> +++ b/accel/tcg/tcg-runtime-gvec.c

> @@ -874,7 +874,7 @@ void HELPER(gvec_sar8v)(void *d, void *a, void *b,

uint32_t desc)
>      intptr_t oprsz = simd_oprsz(desc);

>      intptr_t i;

>

> -    for (i = 0; i < oprsz; i += sizeof(vec8)) {

> +    for (i = 0; i < oprsz; i += sizeof(int8_t)) {

>          uint8_t sh = *(uint8_t *)(b + i) & 7;

>          *(int8_t *)(d + i) = *(int8_t *)(a + i) >> sh;

>      }

> @@ -898,7 +898,7 @@ void HELPER(gvec_sar32v)(void *d, void *a, void *b,

uint32_t desc)
>      intptr_t oprsz = simd_oprsz(desc);

>      intptr_t i;

>

> -    for (i = 0; i < oprsz; i += sizeof(vec32)) {

> +    for (i = 0; i < oprsz; i += sizeof(int32_t)) {

>          uint8_t sh = *(uint32_t *)(b + i) & 31;

>          *(int32_t *)(d + i) = *(int32_t *)(a + i) >> sh;

>      }

> @@ -910,7 +910,7 @@ void HELPER(gvec_sar64v)(void *d, void *a, void *b,

uint32_t desc)
>      intptr_t oprsz = simd_oprsz(desc);

>      intptr_t i;

>

> -    for (i = 0; i < oprsz; i += sizeof(vec64)) {

> +    for (i = 0; i < oprsz; i += sizeof(int64_t)) {

>          uint8_t sh = *(uint64_t *)(b + i) & 63;

>          *(int64_t *)(d + i) = *(int64_t *)(a + i) >> sh;

>      }

> --

> 2.17.1

>

>
Aleksandar Markovic June 14, 2019, 6:03 a.m. UTC | #2 
On Jun 14, 2019 7:22 AM, "Aleksandar Markovic" <aleksandar.m.mail@gmail.com>
wrote:
>

>

> On Jun 13, 2019 6:16 AM, "Richard Henderson" <richard.henderson@linaro.org>

wrote:
> >


I just noticed that this patch is already applied to the upstream - no pull
request, no waiting time for community review -

> > The loop is written with scalars, not vectors.

> > Use the correct type when incrementing.

> >

> > Fixes: 5ee5c14cacd

> > Reported-by: Laurent Vivier <lvivier@redhat.com>

> > Tested-by: Laurent Vivier <lvivier@redhat.com>

> > Reviewed-by: Laurent Vivier <lvivier@redhat.com>

> > Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

> > ---

>

> This is certainly not a case of “typo” (which would mean that you

accidentally typed “vec8” instead od “int8_t”). So, change the title to
“tcg: Fix loop step in
> helper_gvec_sar{8, 32, 64}v” or similar.

>

> >  accel/tcg/tcg-runtime-gvec.c | 6 +++---

> >  1 file changed, 3 insertions(+), 3 deletions(-)

> >

> > diff --git a/accel/tcg/tcg-runtime-gvec.c b/accel/tcg/tcg-runtime-gvec.c

> > index 3b6052fe97..51cb29ca79 100644

> > --- a/accel/tcg/tcg-runtime-gvec.c

> > +++ b/accel/tcg/tcg-runtime-gvec.c

> > @@ -874,7 +874,7 @@ void HELPER(gvec_sar8v)(void *d, void *a, void *b,

uint32_t desc)
> >      intptr_t oprsz = simd_oprsz(desc);

> >      intptr_t i;

> >

> > -    for (i = 0; i < oprsz; i += sizeof(vec8)) {

> > +    for (i = 0; i < oprsz; i += sizeof(int8_t)) {

> >          uint8_t sh = *(uint8_t *)(b + i) & 7;

> >          *(int8_t *)(d + i) = *(int8_t *)(a + i) >> sh;

> >      }

> > @@ -898,7 +898,7 @@ void HELPER(gvec_sar32v)(void *d, void *a, void *b,

uint32_t desc)
> >      intptr_t oprsz = simd_oprsz(desc);

> >      intptr_t i;

> >

> > -    for (i = 0; i < oprsz; i += sizeof(vec32)) {

> > +    for (i = 0; i < oprsz; i += sizeof(int32_t)) {

> >          uint8_t sh = *(uint32_t *)(b + i) & 31;

> >          *(int32_t *)(d + i) = *(int32_t *)(a + i) >> sh;

> >      }

> > @@ -910,7 +910,7 @@ void HELPER(gvec_sar64v)(void *d, void *a, void *b,

uint32_t desc)
> >      intptr_t oprsz = simd_oprsz(desc);

> >      intptr_t i;

> >

> > -    for (i = 0; i < oprsz; i += sizeof(vec64)) {

> > +    for (i = 0; i < oprsz; i += sizeof(int64_t)) {

> >          uint8_t sh = *(uint64_t *)(b + i) & 63;

> >          *(int64_t *)(d + i) = *(int64_t *)(a + i) >> sh;

> >      }

> > --

> > 2.17.1

> >

> >
Aleksandar Markovic June 14, 2019, 6:09 a.m. UTC | #3 
On Jun 14, 2019 8:03 AM, "Aleksandar Markovic" <aleksandar.m.mail@gmail.com>
wrote:
>

>

> On Jun 14, 2019 7:22 AM, "Aleksandar Markovic" <

aleksandar.m.mail@gmail.com> wrote:
> >

> >

> > On Jun 13, 2019 6:16 AM, "Richard Henderson" <

richard.henderson@linaro.org> wrote:
> > >

>

> I just noticed that this patch is already applied to the upstream - no

pull request, no waiting time for community review -
>


Peter, would you please explain to me what was going on with this patch? I
am truly confused, since all rules and procedures seem to be somehow
circumvented, don't they?

Yours,
Aleksandar

> > > The loop is written with scalars, not vectors.

> > > Use the correct type when incrementing.

> > >

> > > Fixes: 5ee5c14cacd

> > > Reported-by: Laurent Vivier <lvivier@redhat.com>

> > > Tested-by: Laurent Vivier <lvivier@redhat.com>

> > > Reviewed-by: Laurent Vivier <lvivier@redhat.com>

> > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

> > > ---

> >

> > This is certainly not a case of “typo” (which would mean that you

accidentally typed “vec8” instead od “int8_t”). So, change the title to
“tcg: Fix loop step in
> > helper_gvec_sar{8, 32, 64}v” or similar.

> >

> > >  accel/tcg/tcg-runtime-gvec.c | 6 +++---

> > >  1 file changed, 3 insertions(+), 3 deletions(-)

> > >

> > > diff --git a/accel/tcg/tcg-runtime-gvec.c

b/accel/tcg/tcg-runtime-gvec.c
> > > index 3b6052fe97..51cb29ca79 100644

> > > --- a/accel/tcg/tcg-runtime-gvec.c

> > > +++ b/accel/tcg/tcg-runtime-gvec.c

> > > @@ -874,7 +874,7 @@ void HELPER(gvec_sar8v)(void *d, void *a, void

*b, uint32_t desc)
> > >      intptr_t oprsz = simd_oprsz(desc);

> > >      intptr_t i;

> > >

> > > -    for (i = 0; i < oprsz; i += sizeof(vec8)) {

> > > +    for (i = 0; i < oprsz; i += sizeof(int8_t)) {

> > >          uint8_t sh = *(uint8_t *)(b + i) & 7;

> > >          *(int8_t *)(d + i) = *(int8_t *)(a + i) >> sh;

> > >      }

> > > @@ -898,7 +898,7 @@ void HELPER(gvec_sar32v)(void *d, void *a, void

*b, uint32_t desc)
> > >      intptr_t oprsz = simd_oprsz(desc);

> > >      intptr_t i;

> > >

> > > -    for (i = 0; i < oprsz; i += sizeof(vec32)) {

> > > +    for (i = 0; i < oprsz; i += sizeof(int32_t)) {

> > >          uint8_t sh = *(uint32_t *)(b + i) & 31;

> > >          *(int32_t *)(d + i) = *(int32_t *)(a + i) >> sh;

> > >      }

> > > @@ -910,7 +910,7 @@ void HELPER(gvec_sar64v)(void *d, void *a, void

*b, uint32_t desc)
> > >      intptr_t oprsz = simd_oprsz(desc);

> > >      intptr_t i;

> > >

> > > -    for (i = 0; i < oprsz; i += sizeof(vec64)) {

> > > +    for (i = 0; i < oprsz; i += sizeof(int64_t)) {

> > >          uint8_t sh = *(uint64_t *)(b + i) & 63;

> > >          *(int64_t *)(d + i) = *(int64_t *)(a + i) >> sh;

> > >      }

> > > --

> > > 2.17.1

> > >

> > >
Peter Maydell June 14, 2019, 8:33 a.m. UTC | #4 
On Fri, 14 Jun 2019 at 07:09, Aleksandar Markovic
<aleksandar.m.mail@gmail.com> wrote:
> Peter, would you please explain to me what was going on with this patch?


The original patch was posted to the list on the 7th June; it got
reviewed and tested by Laurent. This email accidentally has "PATCH"
in the subject line instead of "PULL" but it's just part of the pull
request that got it into master -- that's just an accidental
error by Richard when he put the pull request together.

thanks
-- PMM
diff mbox series

Patch

diff --git a/accel/tcg/tcg-runtime-gvec.c b/accel/tcg/tcg-runtime-gvec.c
index 3b6052fe97..51cb29ca79 100644
--- a/accel/tcg/tcg-runtime-gvec.c
+++ b/accel/tcg/tcg-runtime-gvec.c
@@ -874,7 +874,7 @@  void HELPER(gvec_sar8v)(void *d, void *a, void *b, uint32_t desc)
     intptr_t oprsz = simd_oprsz(desc);
     intptr_t i;
 
-    for (i = 0; i < oprsz; i += sizeof(vec8)) {
+    for (i = 0; i < oprsz; i += sizeof(int8_t)) {
         uint8_t sh = *(uint8_t *)(b + i) & 7;
         *(int8_t *)(d + i) = *(int8_t *)(a + i) >> sh;
     }
@@ -898,7 +898,7 @@  void HELPER(gvec_sar32v)(void *d, void *a, void *b, uint32_t desc)
     intptr_t oprsz = simd_oprsz(desc);
     intptr_t i;
 
-    for (i = 0; i < oprsz; i += sizeof(vec32)) {
+    for (i = 0; i < oprsz; i += sizeof(int32_t)) {
         uint8_t sh = *(uint32_t *)(b + i) & 31;
         *(int32_t *)(d + i) = *(int32_t *)(a + i) >> sh;
     }
@@ -910,7 +910,7 @@  void HELPER(gvec_sar64v)(void *d, void *a, void *b, uint32_t desc)
     intptr_t oprsz = simd_oprsz(desc);
     intptr_t i;
 
-    for (i = 0; i < oprsz; i += sizeof(vec64)) {
+    for (i = 0; i < oprsz; i += sizeof(int64_t)) {
         uint8_t sh = *(uint64_t *)(b + i) & 63;
         *(int64_t *)(d + i) = *(int64_t *)(a + i) >> sh;
     }