| Message ID | 20190301115413.27153-4-david@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | s390x/tcg: Vector Instruction Support Part 1 | expand |
On 3/1/19 3:53 AM, David Hildenbrand wrote: > +#ifndef S390X_VEC_H > +#define S390X_VEC_H > + > +typedef union S390Vector { > + uint64_t doubleword[2]; > + uint32_t word[4]; > + uint16_t halfword[8]; > + uint8_t byte[16]; > +} S390Vector; > + > +uint8_t s390_vec_read_element8(const S390Vector *v, uint8_t enr); > +uint16_t s390_vec_read_element16(const S390Vector *v, uint8_t enr); > +uint32_t s390_vec_read_element32(const S390Vector *v, uint8_t enr); > +uint64_t s390_vec_read_element64(const S390Vector *v, uint8_t enr); > +void s390_vec_write_element8(S390Vector *v, uint8_t enr, uint8_t data); > +void s390_vec_write_element16(S390Vector *v, uint8_t enr, uint16_t data); > +void s390_vec_write_element32(S390Vector *v, uint8_t enr, uint32_t data); > +void s390_vec_write_element64(S390Vector *v, uint8_t enr, uint64_t data); > + These, I think, should be static inline. Certainly I think they're too small to warrant a real call across modules, as you appear to be preparing for... I'm not sure why else they're being declared outside vec_helper.c? r~
On 01.03.19 17:09, Richard Henderson wrote: > On 3/1/19 3:53 AM, David Hildenbrand wrote: >> +#ifndef S390X_VEC_H >> +#define S390X_VEC_H >> + >> +typedef union S390Vector { >> + uint64_t doubleword[2]; >> + uint32_t word[4]; >> + uint16_t halfword[8]; >> + uint8_t byte[16]; >> +} S390Vector; >> + >> +uint8_t s390_vec_read_element8(const S390Vector *v, uint8_t enr); >> +uint16_t s390_vec_read_element16(const S390Vector *v, uint8_t enr); >> +uint32_t s390_vec_read_element32(const S390Vector *v, uint8_t enr); >> +uint64_t s390_vec_read_element64(const S390Vector *v, uint8_t enr); >> +void s390_vec_write_element8(S390Vector *v, uint8_t enr, uint8_t data); >> +void s390_vec_write_element16(S390Vector *v, uint8_t enr, uint16_t data); >> +void s390_vec_write_element32(S390Vector *v, uint8_t enr, uint32_t data); >> +void s390_vec_write_element64(S390Vector *v, uint8_t enr, uint64_t data); >> + > > These, I think, should be static inline. Certainly I think they're too small > to warrant a real call across modules, as you appear to be preparing for... I'm wondering, won't link-time optimization properly handle this? (so far the theory I know of) But I can move these completely into the head. > > I'm not sure why else they're being declared outside vec_helper.c? > We'll have vec_int_helper.c, vec_fpu_helper.c vec_string_helper.c .... Thanks!
On 3/1/19 8:13 AM, David Hildenbrand wrote: >> These, I think, should be static inline. Certainly I think they're too small >> to warrant a real call across modules, as you appear to be preparing for... > > I'm wondering, won't link-time optimization properly handle this? (so > far the theory I know of) It would, but who does that? It's certainly not in -O2. I suspect -flto is a set of -Werror and other sorts of compiler appeasement changes beyond what anyone has tried. r~
On 01.03.19 17:13, David Hildenbrand wrote: > On 01.03.19 17:09, Richard Henderson wrote: >> On 3/1/19 3:53 AM, David Hildenbrand wrote: >>> +#ifndef S390X_VEC_H >>> +#define S390X_VEC_H >>> + >>> +typedef union S390Vector { >>> + uint64_t doubleword[2]; >>> + uint32_t word[4]; >>> + uint16_t halfword[8]; >>> + uint8_t byte[16]; >>> +} S390Vector; >>> + >>> +uint8_t s390_vec_read_element8(const S390Vector *v, uint8_t enr); >>> +uint16_t s390_vec_read_element16(const S390Vector *v, uint8_t enr); >>> +uint32_t s390_vec_read_element32(const S390Vector *v, uint8_t enr); >>> +uint64_t s390_vec_read_element64(const S390Vector *v, uint8_t enr); >>> +void s390_vec_write_element8(S390Vector *v, uint8_t enr, uint8_t data); >>> +void s390_vec_write_element16(S390Vector *v, uint8_t enr, uint16_t data); >>> +void s390_vec_write_element32(S390Vector *v, uint8_t enr, uint32_t data); >>> +void s390_vec_write_element64(S390Vector *v, uint8_t enr, uint64_t data); >>> + >> >> These, I think, should be static inline. Certainly I think they're too small >> to warrant a real call across modules, as you appear to be preparing for... > > I'm wondering, won't link-time optimization properly handle this? (so > far the theory I know of) > > But I can move these completely into the head. Just checked the binary, definetly not optimized during link time. Will make them static inline. Thanks!
On 01.03.19 17:16, Richard Henderson wrote: > On 3/1/19 8:13 AM, David Hildenbrand wrote: >>> These, I think, should be static inline. Certainly I think they're too small >>> to warrant a real call across modules, as you appear to be preparing for... >> >> I'm wondering, won't link-time optimization properly handle this? (so >> far the theory I know of) > > It would, but who does that? It's certainly not in -O2. I know that GCC and LLVM do it, but as you correctly state, most probably not for O2.
diff --git a/target/s390x/Makefile.objs b/target/s390x/Makefile.objs index 22a9a9927a..68eeee3d2f 100644 --- a/target/s390x/Makefile.objs +++ b/target/s390x/Makefile.objs @@ -1,6 +1,7 @@ obj-y += cpu.o cpu_models.o cpu_features.o gdbstub.o interrupt.o helper.o obj-$(CONFIG_TCG) += translate.o cc_helper.o excp_helper.o fpu_helper.o obj-$(CONFIG_TCG) += int_helper.o mem_helper.o misc_helper.o crypto_helper.o +obj-$(CONFIG_TCG) += vec_helper.o obj-$(CONFIG_SOFTMMU) += machine.o ioinst.o arch_dump.o mmu_helper.o diag.o obj-$(CONFIG_SOFTMMU) += sigp.o obj-$(CONFIG_KVM) += kvm.o diff --git a/target/s390x/vec.h b/target/s390x/vec.h new file mode 100644 index 0000000000..c03be1a9c9 --- /dev/null +++ b/target/s390x/vec.h @@ -0,0 +1,31 @@ +/* + * QEMU TCG support -- s390x vector utilitites + * + * Copyright (C) 2019 Red Hat Inc + * + * Authors: + * David Hildenbrand <david@redhat.com> + * + * This work is licensed under the terms of the GNU GPL, version 2 or later. + * See the COPYING file in the top-level directory. + */ +#ifndef S390X_VEC_H +#define S390X_VEC_H + +typedef union S390Vector { + uint64_t doubleword[2]; + uint32_t word[4]; + uint16_t halfword[8]; + uint8_t byte[16]; +} S390Vector; + +uint8_t s390_vec_read_element8(const S390Vector *v, uint8_t enr); +uint16_t s390_vec_read_element16(const S390Vector *v, uint8_t enr); +uint32_t s390_vec_read_element32(const S390Vector *v, uint8_t enr); +uint64_t s390_vec_read_element64(const S390Vector *v, uint8_t enr); +void s390_vec_write_element8(S390Vector *v, uint8_t enr, uint8_t data); +void s390_vec_write_element16(S390Vector *v, uint8_t enr, uint16_t data); +void s390_vec_write_element32(S390Vector *v, uint8_t enr, uint32_t data); +void s390_vec_write_element64(S390Vector *v, uint8_t enr, uint64_t data); + +#endif /* S390X_VEC_H */ diff --git a/target/s390x/vec_helper.c b/target/s390x/vec_helper.c new file mode 100644 index 0000000000..3e21e440ba --- /dev/null +++ b/target/s390x/vec_helper.c @@ -0,0 +1,90 @@ +/* + * QEMU TCG support -- s390x vector support instructions and utilitites + * + * Copyright (C) 2019 Red Hat Inc + * + * Authors: + * David Hildenbrand <david@redhat.com> + * + * This work is licensed under the terms of the GNU GPL, version 2 or later. + * See the COPYING file in the top-level directory. + */ +#include "qemu/osdep.h" +#include "qemu-common.h" +#include "vec.h" +#include "tcg/tcg.h" + +/* + * Each vector is stored as two 64bit host values. So when talking about + * byte/halfword/word numbers, we have to take care of proper translation + * between element numbers. + * + * Big Endian (target/possible host) + * B: [ 0][ 1][ 2][ 3][ 4][ 5][ 6][ 7] - [ 8][ 9][10][11][12][13][14][15] + * HW: [ 0][ 1][ 2][ 3] - [ 4][ 5][ 6][ 7] + * W: [ 0][ 1] - [ 2][ 3] + * DW: [ 0] - [ 1] + * + * Little Endian (possible host) + * B: [ 7][ 6][ 5][ 4][ 3][ 2][ 1][ 0] - [15][14][13][12][11][10][ 9][ 8] + * HW: [ 3][ 2][ 1][ 0] - [ 7][ 6][ 5][ 4] + * W: [ 1][ 0] - [ 3][ 2] + * DW: [ 0] - [ 1] + */ +#ifndef HOST_WORDS_BIGENDIAN +#define H1(x) ((x) ^ 7) +#define H2(x) ((x) ^ 3) +#define H4(x) ((x) ^ 1) +#else +#define H1(x) (x) +#define H2(x) (x) +#define H4(x) (x) +#endif + +uint8_t s390_vec_read_element8(const S390Vector *v, uint8_t enr) +{ + g_assert(enr < 16); + return v->byte[H1(enr)]; +} + +uint16_t s390_vec_read_element16(const S390Vector *v, uint8_t enr) +{ + g_assert(enr < 8); + return v->halfword[H2(enr)]; +} + +uint32_t s390_vec_read_element32(const S390Vector *v, uint8_t enr) +{ + g_assert(enr < 4); + return v->word[H4(enr)]; +} + +uint64_t s390_vec_read_element64(const S390Vector *v, uint8_t enr) +{ + g_assert(enr < 2); + return v->doubleword[enr]; +} + +void s390_vec_write_element8(S390Vector *v, uint8_t enr, uint8_t data) +{ + g_assert(enr < 16); + v->byte[H1(enr)] = data; +} + +void s390_vec_write_element16(S390Vector *v, uint8_t enr, uint16_t data) +{ + g_assert(enr < 8); + v->halfword[H2(enr)] = data; +} + +void s390_vec_write_element32(S390Vector *v, uint8_t enr, uint32_t data) +{ + g_assert(enr < 4); + v->word[H4(enr)] = data; +} + +void s390_vec_write_element64(S390Vector *v, uint8_t enr, uint64_t data) +{ + g_assert(enr < 2); + v->doubleword[enr] = data; +}
We'll have to read/write vector elements quite frequently from helpers. The tricky bit is properly taking care of endianess. Handle it similar to aarch64. target/s390x/vec_helper.c will later also contain vector support instruction helpers. Signed-off-by: David Hildenbrand <david@redhat.com> --- target/s390x/Makefile.objs | 1 + target/s390x/vec.h | 31 +++++++++++++ target/s390x/vec_helper.c | 90 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 122 insertions(+) create mode 100644 target/s390x/vec.h create mode 100644 target/s390x/vec_helper.c