Re: [Qemu-devel] [PATCH 02/21] target-s390x: split FPU ops
Am 07.09.2012 06:26, schrieb Alexander Graf: Quoting Richard Henderson r...@twiddle.net: On 09/06/2012 11:42 AM, Alexander Graf wrote: Richard, while at it, could you please check the s390x tcg target? Running any target on there seems to break in the TLB code for me. I did successfully run a simple linux-user test directly off blue's patch set. It exercised a bit of fp and system calls (printf). I don't have a system environment set up at the moment... Ah, I am referring to s390x host code. Running qemu-system-x86_64 on s390x is what breaks for me. If, e.g., arm works on master that might rather point to tcg/s390x/ CONFIG_PASS_AREG0 mode. Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH 02/21] target-s390x: split FPU ops
On Fri, Sep 07, 2012 at 04:30:51PM +0200, Andreas Färber wrote: Am 07.09.2012 06:26, schrieb Alexander Graf: Quoting Richard Henderson r...@twiddle.net: On 09/06/2012 11:42 AM, Alexander Graf wrote: Richard, while at it, could you please check the s390x tcg target? Running any target on there seems to break in the TLB code for me. I did successfully run a simple linux-user test directly off blue's patch set. It exercised a bit of fp and system calls (printf). I don't have a system environment set up at the moment... Ah, I am referring to s390x host code. Running qemu-system-x86_64 on s390x is what breaks for me. If, e.g., arm works on master that might rather point to tcg/s390x/ CONFIG_PASS_AREG0 mode. This is likely the case. The register shift code in CONFIG_PASS_AREG0 case uses 3 registers for stores and 4 for loads. It should be the reverse. -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurel...@aurel32.net http://www.aurel32.net
Re: [Qemu-devel] [PATCH 02/21] target-s390x: split FPU ops
On 05.09.2012, at 11:34, Richard Henderson wrote: On 09/04/2012 08:46 PM, Alexander Graf wrote: So that means your rewrite is based on this series and just fixes it up? Does that mean if I apply this patch, you will be all happy? It is not (yet) based on this series. But I will be happy if you apply it, since it's easier for me to rebase off master than find an external tree. Richard, while at it, could you please check the s390x tcg target? Running any target on there seems to break in the TLB code for me. I'm mostly surprised by the env value. Why is that a 32-bit pointer? agraf@s390t27:/home/agraf/git/qemu gdb --args ./x86_64-softmmu/qemu-system-x86_64 -nographic GNU gdb (GDB) SUSE (7.0-0.4.16) Copyright (C) 2009 Free Software Foundation, Inc. License GPLv3+: GNU GPL version 3 or later http://gnu.org/licenses/gpl.html This is free software: you are free to change and redistribute it. There is NO WARRANTY, to the extent permitted by law. Type show copying and show warranty for details. This GDB was configured as s390x-suse-linux. For bug reporting instructions, please see: http://www.gnu.org/software/gdb/bugs/... Reading symbols from /busu/home/agraf/git/qemu/x86_64-softmmu/qemu-system-x86_64...done. (gdb) (gdb) r Starting program: /busu/home/agraf/git/qemu/x86_64-softmmu/qemu-system-x86_64 -nographic Missing separate debuginfo for /usr/lib64/libgthread-2.0.so.0 Try: zypper install -C debuginfo(build-id)=912d1dd1cc78c1a77aec3a03f332a4c3496f5f86 Missing separate debuginfo for /usr/lib64/libglib-2.0.so.0 Try: zypper install -C debuginfo(build-id)=954bad2ed93e5ef700ba03fcd4f6c00e7e0c9d30 Missing separate debuginfo for /usr/lib64/libcurl.so.4 Try: zypper install -C debuginfo(build-id)=05142779be2ae2320d2b0bb942826156ae5c5d81 Missing separate debuginfo for /lib64/libncurses.so.5 Try: zypper install -C debuginfo(build-id)=8091f2982d62f10a40de0e2cd7f2bf74a51f27dc Missing separate debuginfo for /lib64/libuuid.so.1 Try: zypper install -C debuginfo(build-id)=13fbf62f145bc4386a0f4bff001f7dd01c8d78c2 Missing separate debuginfo for /usr/lib64/libpng12.so.0 Try: zypper install -C debuginfo(build-id)=0af6bdbd02f42fea039cf733c79c8573f509fca6 Missing separate debuginfo for /usr/lib64/libjpeg.so.62 Try: zypper install -C debuginfo(build-id)=af2dbf4b5dca2ceb485194bb67411269cb6375a5 Missing separate debuginfo for /usr/lib64/libSDL-1.2.so.0 Try: zypper install -C debuginfo(build-id)=56ac12212e5cc91d4fcc5ea1f68f7bd027587950 [Thread debugging using libthread_db enabled] Missing separate debuginfo for /usr/lib64/libX11.so.6 Try: zypper install -C debuginfo(build-id)=043834a023c164696ce660041637a4f8b56e1f20 Missing separate debuginfo for /lib64/libaio.so.1 Try: zypper install -C debuginfo(build-id)=94043f928d23ab60fa78117484f77e1f27212fcc Missing separate debuginfo for /lib64/libz.so.1 Try: zypper install -C debuginfo(build-id)=5340452e3b691d56fbd88258e8236f0baf6e7f54 Missing separate debuginfo for /usr/lib64/libpcre.so.0 Try: zypper install -C debuginfo(build-id)=dfef694ea1c5f6e5229a26bd1b7679a4b47b6b45 Missing separate debuginfo for /usr/lib64/libidn.so.11 Try: zypper install -C debuginfo(build-id)=5d726f3349ab522b5f19d6dcb86bca442a0826e4 Missing separate debuginfo for /usr/lib64/libssl.so.0.9.8 Try: zypper install -C debuginfo(build-id)=f2b46706fcbc46a3d2bc436e23fd61afc5c68042 Missing separate debuginfo for /usr/lib64/libcrypto.so.0.9.8 Try: zypper install -C debuginfo(build-id)=e8df586992c4aaa050278f62546b8bee2071a72f Missing separate debuginfo for /usr/lib64/libldap-2.4.so.2 Try: zypper install -C debuginfo(build-id)=5b0599064e1c76fad7b3a3a9a9b4a1290a9f31a3 Missing separate debuginfo for /usr/lib64/libgssapi_krb5.so.2 Try: zypper install -C debuginfo(build-id)=76ad9b9a23735144169c1e5bc554f6b4ceea7a9e Missing separate debuginfo for /usr/lib64/libkrb5.so.3 Try: zypper install -C debuginfo(build-id)=69277cfebb420879ae3a613f8c527bec87944723 Missing separate debuginfo for /usr/lib64/libk5crypto.so.3 Try: zypper install -C debuginfo(build-id)=93cc48f64e299f939fa169d714a0124cbab52976 Missing separate debuginfo for /lib64/libcom_err.so.2 Try: zypper install -C debuginfo(build-id)=09b7c836eb5edfad54802a9f9bea475f49758324 Missing separate debuginfo for /lib64/libkeyutils.so.1 Try: zypper install -C debuginfo(build-id)=454eb6d36012309dd468b31a37cdb58bf737ceaf Missing separate debuginfo for /usr/lib64/libXext.so.6 Try: zypper install -C debuginfo(build-id)=1fecbb7e295db9aa8a87e904df04e64dc03a38d1 Missing separate debuginfo for /usr/lib64/libXrandr.so.2 Try: zypper install -C debuginfo(build-id)=8e8ab17f4153f6077fde8e44994ebe8667cfca6d Missing separate debuginfo for /usr/lib64/libXrender.so.1 Try: zypper install -C debuginfo(build-id)=66994ad09a7123bc4aba4c05bf400d2b6bcf41ae Missing separate debuginfo for /usr/lib64/libxcb-xlib.so.0 Try: zypper install -C debuginfo(build-id)=ee68d8a43e76db5e57e128bc147776c11399c40d Missing separate debuginfo for /usr/lib64/libxcb.so.1 Try: zypper install
Re: [Qemu-devel] [PATCH 02/21] target-s390x: split FPU ops
On 09/06/2012 11:42 AM, Alexander Graf wrote: Richard, while at it, could you please check the s390x tcg target? Running any target on there seems to break in the TLB code for me. I did successfully run a simple linux-user test directly off blue's patch set. It exercised a bit of fp and system calls (printf). I don't have a system environment set up at the moment... I'm mostly surprised by the env value. Why is that a 32-bit pointer? It depends on where objects get loaded in memory of course. I don't think there's anything particularly surprising about an object loaded into the load 32-bits. That mmu_idx on the other hand... one begins to wonder if the debugger is actually lying to you. r~
Re: [Qemu-devel] [PATCH 02/21] target-s390x: split FPU ops
Quoting Richard Henderson r...@twiddle.net: On 09/06/2012 11:42 AM, Alexander Graf wrote: Richard, while at it, could you please check the s390x tcg target? Running any target on there seems to break in the TLB code for me. I did successfully run a simple linux-user test directly off blue's patch set. It exercised a bit of fp and system calls (printf). I don't have a system environment set up at the moment... Ah, I am referring to s390x host code. Running qemu-system-x86_64 on s390x is what breaks for me. Alex I'm mostly surprised by the env value. Why is that a 32-bit pointer? It depends on where objects get loaded in memory of course. I don't think there's anything particularly surprising about an object loaded into the load 32-bits. That mmu_idx on the other hand... one begins to wonder if the debugger is actually lying to you. r~
Re: [Qemu-devel] [PATCH 02/21] target-s390x: split FPU ops
On 09/04/2012 08:46 PM, Alexander Graf wrote: So that means your rewrite is based on this series and just fixes it up? Does that mean if I apply this patch, you will be all happy? It is not (yet) based on this series. But I will be happy if you apply it, since it's easier for me to rebase off master than find an external tree. r~
Re: [Qemu-devel] [PATCH 02/21] target-s390x: split FPU ops
On 05.09.2012, at 11:34, Richard Henderson wrote: On 09/04/2012 08:46 PM, Alexander Graf wrote: So that means your rewrite is based on this series and just fixes it up? Does that mean if I apply this patch, you will be all happy? It is not (yet) based on this series. But I will be happy if you apply it, since it's easier for me to rebase off master than find an external tree. Ok, I need a bit more concentration that I can get now for that though and will hop on a reasonably long plane flight in a few hours. Will apply / review it afterwards, though I don't expect too many problems. Thus you might be easier off to rebase your series early on on top of Blue's patch set. Alex
Re: [Qemu-devel] [PATCH 02/21] target-s390x: split FPU ops
On 09/02/2012 10:33 AM, Blue Swirl wrote: +/* fpu_helper.c */ +uint32_t set_cc_f32(float32 v1, float32 v2); +uint32_t set_cc_f64(float64 v1, float64 v2); +uint32_t set_cc_nz_f32(float32 v); +uint32_t set_cc_nz_f64(float64 v); + I think that the CC handling should stay together, regardless of FPU-ness. These functions are quite small and can be usefully inlined by the compiler. OTOH, this is much easier to do with my translate.c rewrite, so maybe I'll just take responsibility for moving them back... r~
Re: [Qemu-devel] [PATCH 02/21] target-s390x: split FPU ops
On Tue, Sep 4, 2012 at 6:42 PM, Richard Henderson r...@twiddle.net wrote: On 09/02/2012 10:33 AM, Blue Swirl wrote: +/* fpu_helper.c */ +uint32_t set_cc_f32(float32 v1, float32 v2); +uint32_t set_cc_f64(float64 v1, float64 v2); +uint32_t set_cc_nz_f32(float32 v); +uint32_t set_cc_nz_f64(float64 v); + I think that the CC handling should stay together, regardless of FPU-ness. These functions are quite small and can be usefully inlined by the compiler. OTOH, this is much easier to do with my translate.c rewrite, so maybe I'll just take responsibility for moving them back... The problem is that these are used by some FPU ops as well as condition code ops. Maybe it's better to move them to cpu.h as inline functions? It could be also possible to upgrade condition code functions to full helpers, that could help implementing lazy condition code evaluation. I noticed that the helpers don't use TCGv registers for register access but instead the helpers access env-regs[] and env-fregs[] directly, this probably would need to be changed too. r~
Re: [Qemu-devel] [PATCH 02/21] target-s390x: split FPU ops
On 09/04/2012 12:40 PM, Blue Swirl wrote: On Tue, Sep 4, 2012 at 6:42 PM, Richard Henderson r...@twiddle.net wrote: On 09/02/2012 10:33 AM, Blue Swirl wrote: +/* fpu_helper.c */ +uint32_t set_cc_f32(float32 v1, float32 v2); +uint32_t set_cc_f64(float64 v1, float64 v2); +uint32_t set_cc_nz_f32(float32 v); +uint32_t set_cc_nz_f64(float64 v); + I think that the CC handling should stay together, regardless of FPU-ness. These functions are quite small and can be usefully inlined by the compiler. OTOH, this is much easier to do with my translate.c rewrite, so maybe I'll just take responsibility for moving them back... The problem is that these are used by some FPU ops as well as condition code ops. Maybe it's better to move them to cpu.h as inline functions? Maybe... It could be also possible to upgrade condition code functions to full helpers, that could help implementing lazy condition code evaluation. Done and ... I noticed that the helpers don't use TCGv registers for register access but instead the helpers access env-regs[] and env-fregs[] directly, this probably would need to be changed too. done, in my rewrite. Except for float128, where we can't return such by value inside TCG. And, annoyingly, s390 float128 values are held in %fN/%fN+2, so the values are not contiguous in memory. I momentarily considered passing a pointer to %fN, letting the helper access f[0]/f[2], but in the end I didn't consider that any better than just passing the integer N. So after the rewrite, set_cc_nz_f32/64 are not referenced from the fp helpers directly. We still reference set_cc_nz_f128 from ADXBR and SDXBR. r~
Re: [Qemu-devel] [PATCH 02/21] target-s390x: split FPU ops
On 04.09.2012, at 18:03, Richard Henderson wrote: On 09/04/2012 12:40 PM, Blue Swirl wrote: On Tue, Sep 4, 2012 at 6:42 PM, Richard Henderson r...@twiddle.net wrote: On 09/02/2012 10:33 AM, Blue Swirl wrote: +/* fpu_helper.c */ +uint32_t set_cc_f32(float32 v1, float32 v2); +uint32_t set_cc_f64(float64 v1, float64 v2); +uint32_t set_cc_nz_f32(float32 v); +uint32_t set_cc_nz_f64(float64 v); + I think that the CC handling should stay together, regardless of FPU-ness. These functions are quite small and can be usefully inlined by the compiler. OTOH, this is much easier to do with my translate.c rewrite, so maybe I'll just take responsibility for moving them back... The problem is that these are used by some FPU ops as well as condition code ops. Maybe it's better to move them to cpu.h as inline functions? Maybe... It could be also possible to upgrade condition code functions to full helpers, that could help implementing lazy condition code evaluation. Done and ... I noticed that the helpers don't use TCGv registers for register access but instead the helpers access env-regs[] and env-fregs[] directly, this probably would need to be changed too. done, in my rewrite. So that means your rewrite is based on this series and just fixes it up? Does that mean if I apply this patch, you will be all happy? Alex
[Qemu-devel] [PATCH 02/21] target-s390x: split FPU ops
Move floating point instructions to fpu_helper.c.
While exporting some condition code helpers,
avoid duplicate identifier conflict with translate.c.
Remove unused set_cc_nz_f64() in translate.c.
Signed-off-by: Blue Swirl blauwir...@gmail.com
---
target-s390x/Makefile.objs |2 +
target-s390x/cpu.h |6 +
target-s390x/fpu_helper.c | 836
target-s390x/op_helper.c | 802 --
target-s390x/translate.c | 11 +-
5 files changed, 847 insertions(+), 810 deletions(-)
create mode 100644 target-s390x/fpu_helper.c
diff --git a/target-s390x/Makefile.objs b/target-s390x/Makefile.objs
index 80be3bb..23b3bd9 100644
--- a/target-s390x/Makefile.objs
+++ b/target-s390x/Makefile.objs
@@ -1,5 +1,7 @@
obj-y += translate.o op_helper.o helper.o cpu.o interrupt.o
+obj-y += fpu_helper.o
obj-$(CONFIG_SOFTMMU) += machine.o
obj-$(CONFIG_KVM) += kvm.o
$(obj)/op_helper.o: QEMU_CFLAGS += $(HELPER_CFLAGS)
+$(obj)/fpu_helper.o: QEMU_CFLAGS += $(HELPER_CFLAGS)
diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
index 18ac6e3..b4620c5 100644
--- a/target-s390x/cpu.h
+++ b/target-s390x/cpu.h
@@ -999,4 +999,10 @@ static inline void cpu_pc_from_tb(CPUS390XState *env,
TranslationBlock* tb)
env-psw.addr = tb-pc;
}
+/* fpu_helper.c */
+uint32_t set_cc_f32(float32 v1, float32 v2);
+uint32_t set_cc_f64(float64 v1, float64 v2);
+uint32_t set_cc_nz_f32(float32 v);
+uint32_t set_cc_nz_f64(float64 v);
+
#endif
diff --git a/target-s390x/fpu_helper.c b/target-s390x/fpu_helper.c
new file mode 100644
index 000..1389052
--- /dev/null
+++ b/target-s390x/fpu_helper.c
@@ -0,0 +1,836 @@
+/*
+ * S/390 FPU helper routines
+ *
+ * Copyright (c) 2009 Ulrich Hecht
+ * Copyright (c) 2009 Alexander Graf
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see http://www.gnu.org/licenses/.
+ */
+
+#include cpu.h
+#include dyngen-exec.h
+#include helper.h
+
+#if !defined(CONFIG_USER_ONLY)
+#include softmmu_exec.h
+#endif
+
+/* #define DEBUG_HELPER */
+#ifdef DEBUG_HELPER
+#define HELPER_LOG(x...) qemu_log(x)
+#else
+#define HELPER_LOG(x...)
+#endif
+
+static inline int float_comp_to_cc(int float_compare)
+{
+switch (float_compare) {
+case float_relation_equal:
+return 0;
+case float_relation_less:
+return 1;
+case float_relation_greater:
+return 2;
+case float_relation_unordered:
+return 3;
+default:
+cpu_abort(env, unknown return value for float compare\n);
+}
+}
+
+/* condition codes for binary FP ops */
+uint32_t set_cc_f32(float32 v1, float32 v2)
+{
+return float_comp_to_cc(float32_compare_quiet(v1, v2, env-fpu_status));
+}
+
+uint32_t set_cc_f64(float64 v1, float64 v2)
+{
+return float_comp_to_cc(float64_compare_quiet(v1, v2, env-fpu_status));
+}
+
+/* condition codes for unary FP ops */
+uint32_t set_cc_nz_f32(float32 v)
+{
+if (float32_is_any_nan(v)) {
+return 3;
+} else if (float32_is_zero(v)) {
+return 0;
+} else if (float32_is_neg(v)) {
+return 1;
+} else {
+return 2;
+}
+}
+
+uint32_t set_cc_nz_f64(float64 v)
+{
+if (float64_is_any_nan(v)) {
+return 3;
+} else if (float64_is_zero(v)) {
+return 0;
+} else if (float64_is_neg(v)) {
+return 1;
+} else {
+return 2;
+}
+}
+
+static uint32_t set_cc_nz_f128(float128 v)
+{
+if (float128_is_any_nan(v)) {
+return 3;
+} else if (float128_is_zero(v)) {
+return 0;
+} else if (float128_is_neg(v)) {
+return 1;
+} else {
+return 2;
+}
+}
+
+/* convert 32-bit int to 64-bit float */
+void HELPER(cdfbr)(uint32_t f1, int32_t v2)
+{
+HELPER_LOG(%s: converting %d to f%d\n, __func__, v2, f1);
+env-fregs[f1].d = int32_to_float64(v2, env-fpu_status);
+}
+
+/* convert 32-bit int to 128-bit float */
+void HELPER(cxfbr)(uint32_t f1, int32_t v2)
+{
+CPU_QuadU v1;
+
+v1.q = int32_to_float128(v2, env-fpu_status);
+env-fregs[f1].ll = v1.ll.upper;
+env-fregs[f1 + 2].ll = v1.ll.lower;
+}
+
+/* convert 64-bit int to 32-bit float */
+void HELPER(cegbr)(uint32_t f1, int64_t v2)
+{
+HELPER_LOG(%s: converting %ld to f%d\n, __func__, v2, f1);
+env-fregs[f1].l.upper = int64_to_float32(v2, env-fpu_status);
+}
+
+/* convert 64-bit int to 64-bit float */
+void
