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

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(-)

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
>
>

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
> >
> >

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
> > >
> > >

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