| Message ID | 20190225115552.7534-4-david@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | s390x/tcg: Cleanups and refactorings for Vector | expand |
On 25/02/2019 12.55, David Hildenbrand wrote: > We will use s390x speak "Element Size" (es) for MO_8 == 0, MO_16 == 1 > ... Simple rename of variables. > > Signed-off-by: David Hildenbrand <david@redhat.com> > --- > target/s390x/translate.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/target/s390x/translate.c b/target/s390x/translate.c > index c646e50eb3..916508b567 100644 > --- a/target/s390x/translate.c > +++ b/target/s390x/translate.c > @@ -145,10 +145,10 @@ void s390x_translate_init(void) > } > } > > -static inline int vec_reg_offset(uint8_t reg, uint8_t enr, TCGMemOp size) > +static inline int vec_reg_offset(uint8_t reg, uint8_t enr, TCGMemOp es) > { > - const uint8_t es = 1 << size; > - int offs = enr * es; > + const uint8_t bytes = 1 << es; I'd maybe add a comment at the end of above line, saying "/* Convert element size to bytes */" or something similar, so that it is clear to the reader that "es" means "element size"... > + int offs = enr * bytes; > > g_assert(reg < 32); > /* > @@ -173,9 +173,9 @@ static inline int vec_reg_offset(uint8_t reg, uint8_t enr, TCGMemOp size) > * the two 8 byte elements have to be loaded separately. Let's force all > * 16 byte operations to handle it in a special way. > */ > - g_assert(size <= MO_64); > + g_assert(es <= MO_64); > #ifndef HOST_WORDS_BIGENDIAN > - offs ^= (8 - es); > + offs ^= (8 - bytes); > #endif > return offs + offsetof(CPUS390XState, vregs[reg][0].d); > } > Reviewed-by: Thomas Huth <thuth@redhat.com>
On 25.02.19 13:30, Thomas Huth wrote: > On 25/02/2019 12.55, David Hildenbrand wrote: >> We will use s390x speak "Element Size" (es) for MO_8 == 0, MO_16 == 1 >> ... Simple rename of variables. >> >> Signed-off-by: David Hildenbrand <david@redhat.com> >> --- >> target/s390x/translate.c | 10 +++++----- >> 1 file changed, 5 insertions(+), 5 deletions(-) >> >> diff --git a/target/s390x/translate.c b/target/s390x/translate.c >> index c646e50eb3..916508b567 100644 >> --- a/target/s390x/translate.c >> +++ b/target/s390x/translate.c >> @@ -145,10 +145,10 @@ void s390x_translate_init(void) >> } >> } >> >> -static inline int vec_reg_offset(uint8_t reg, uint8_t enr, TCGMemOp size) >> +static inline int vec_reg_offset(uint8_t reg, uint8_t enr, TCGMemOp es) >> { >> - const uint8_t es = 1 << size; >> - int offs = enr * es; >> + const uint8_t bytes = 1 << es; > > I'd maybe add a comment at the end of above line, saying "/* Convert > element size to bytes */" or something similar, so that it is clear to > the reader that "es" means "element size"... Can do, in the actual vector instruction part I have a comment explaining the terminology, but that will be in another file. > >> + int offs = enr * bytes; >> >> g_assert(reg < 32); >> /* >> @@ -173,9 +173,9 @@ static inline int vec_reg_offset(uint8_t reg, uint8_t enr, TCGMemOp size) >> * the two 8 byte elements have to be loaded separately. Let's force all >> * 16 byte operations to handle it in a special way. >> */ >> - g_assert(size <= MO_64); >> + g_assert(es <= MO_64); >> #ifndef HOST_WORDS_BIGENDIAN >> - offs ^= (8 - es); >> + offs ^= (8 - bytes); >> #endif >> return offs + offsetof(CPUS390XState, vregs[reg][0].d); >> } >> > > Reviewed-by: Thomas Huth <thuth@redhat.com> > Thanks!
On 2/25/19 3:55 AM, David Hildenbrand wrote: > We will use s390x speak "Element Size" (es) for MO_8 == 0, MO_16 == 1 > ... Simple rename of variables. > > Signed-off-by: David Hildenbrand <david@redhat.com> > --- > target/s390x/translate.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
diff --git a/target/s390x/translate.c b/target/s390x/translate.c index c646e50eb3..916508b567 100644 --- a/target/s390x/translate.c +++ b/target/s390x/translate.c @@ -145,10 +145,10 @@ void s390x_translate_init(void) } } -static inline int vec_reg_offset(uint8_t reg, uint8_t enr, TCGMemOp size) +static inline int vec_reg_offset(uint8_t reg, uint8_t enr, TCGMemOp es) { - const uint8_t es = 1 << size; - int offs = enr * es; + const uint8_t bytes = 1 << es; + int offs = enr * bytes; g_assert(reg < 32); /* @@ -173,9 +173,9 @@ static inline int vec_reg_offset(uint8_t reg, uint8_t enr, TCGMemOp size) * the two 8 byte elements have to be loaded separately. Let's force all * 16 byte operations to handle it in a special way. */ - g_assert(size <= MO_64); + g_assert(es <= MO_64); #ifndef HOST_WORDS_BIGENDIAN - offs ^= (8 - es); + offs ^= (8 - bytes); #endif return offs + offsetof(CPUS390XState, vregs[reg][0].d); }
We will use s390x speak "Element Size" (es) for MO_8 == 0, MO_16 == 1 ... Simple rename of variables. Signed-off-by: David Hildenbrand <david@redhat.com> --- target/s390x/translate.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)