[Qemu-devel] [PATCH for-1.1 0/3] tcg/ppc: AREG0 support and Darwin fixes

[Qemu-devel] [PATCH for-1.1 0/3] tcg/ppc: AREG0 support and Darwin fixes

[Andreas Färber]

Hello malc,

This series fixes two long-standing issues on Darwin/ppc and, based on your
second patch, fixes the AREG0 mode for Linux and Darwin and thus the ppc build.

Compared to your last patch I have tried to avoid the { int ir; } block, and
I've combined both modes in one code path using ir and a new macro to handle
the alignment differences in a more explicit way.

Tested on Linux and Darwin, using i386 and x86_64 (non-AREG0) as well as sparc
and sparc64 (AREG0). On Darwin I observed a reproducible hang in fsck during
INIT under Debian/sparc, which given the lateness and now unified code paths
(i.e., Haiku/i386 and HelenOS/sparc64 working) I'd attribute to lingering
main loop / signal handling issues.

Regards,
Andreas

Cc: malc <av1474@comtv.ru>

Andreas Färber (3):
  tcg/ppc: Do not overwrite lower address word on Darwin and AIX
  tcg/ppc: Handle _CALL_DARWIN being undefined on Darwin
  tcg/ppc: Fix CONFIG_TCG_PASS_AREG0 mode

 tcg/ppc/tcg-target.c |   54 +++++++++++++++++++++++++++++++++----------------
 1 files changed, 36 insertions(+), 18 deletions(-)

-- 
1.7.7

[Qemu-devel] [PATCH for-1.1 1/3] tcg/ppc: Do not overwrite lower address word on Darwin and AIX

[Andreas Färber]

From: Andreas Färber <andreas.faerber@web.de>

For targets where TARGET_LONG_BITS != 32, i.e. 64-bit guests,
addr_reg is moved to r4. For hosts without TCG_TARGET_CALL_ALIGN_ARGS
either data_reg2 or data_reg or a masked version thereof would overwrite
r4. Place it in r5 instead, matching TCG_TARGET_CALL_ALIGN_ARGS hosts.

This fixes immediate crashes of 64-bit guests observed on Darwin/ppc but
not on Darwin/ppc64.

Signed-off-by: Andreas Färber <andreas.faerber@web.de>
---
 tcg/ppc/tcg-target.c |    4 ----
 1 files changed, 0 insertions(+), 4 deletions(-)

diff --git a/tcg/ppc/tcg-target.c b/tcg/ppc/tcg-target.c
index dc40716..4b85c89 100644
--- a/tcg/ppc/tcg-target.c
+++ b/tcg/ppc/tcg-target.c
@@ -816,11 +816,7 @@ static void tcg_out_qemu_st (TCGContext *s, const TCGArg *args, int opc)
 #else
     tcg_out_mov (s, TCG_TYPE_I32, 3, addr_reg2);
     tcg_out_mov (s, TCG_TYPE_I32, 4, addr_reg);
-#ifdef TCG_TARGET_CALL_ALIGN_ARGS
     ir = 5;
-#else
-    ir = 4;
-#endif
 #endif
 
     switch (opc) {
-- 
1.7.7

[Qemu-devel] [PATCH for-1.1 2/3] tcg/ppc: Handle _CALL_DARWIN being undefined on Darwin

[Andreas Färber]

From: Andreas Färber <andreas.faerber@web.de>

powerpc-apple-darwin9-gcc-4.0.1 (GCC) 4.0.1 (Apple Inc. build 5493)
does not define _CALL_DARWIN, leading to unexpected behavior w.r.t.
register clobbering and stack frame layout.

Define _CALL_DARWIN if necessary.

Signed-off-by: Andreas Färber <andreas.faerber@web.de>
---
 tcg/ppc/tcg-target.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/tcg/ppc/tcg-target.c b/tcg/ppc/tcg-target.c
index 4b85c89..5a651ce 100644
--- a/tcg/ppc/tcg-target.c
+++ b/tcg/ppc/tcg-target.c
@@ -24,6 +24,10 @@
 
 static uint8_t *tb_ret_addr;
 
+#if defined __APPLE__ && !defined _CALL_DARWIN
+#define _CALL_DARWIN
+#endif
+
 #ifdef _CALL_DARWIN
 #define LINKAGE_AREA_SIZE 24
 #define LR_OFFSET 8
-- 
1.7.7

Re: [Qemu-devel] [PATCH for-1.1 2/3] tcg/ppc: Handle _CALL_DARWIN being undefined on Darwin

[Andreas Färber]

Am 07.05.2012 01:46, schrieb Andreas Färber:
> From: Andreas Färber <andreas.faerber@web.de>
> 
> powerpc-apple-darwin9-gcc-4.0.1 (GCC) 4.0.1 (Apple Inc. build 5493)
> does not define _CALL_DARWIN, leading to unexpected behavior w.r.t.
> register clobbering and stack frame layout.

As requested, here's GCC output:

$ gcc --version
powerpc-apple-darwin9-gcc-4.0.1 (GCC) 4.0.1 (Apple Inc. build 5493)
Copyright (C) 2005 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
$ gcc -E -dM -x c /dev/null
#define __DBL_MIN_EXP__ (-1021)
#define __FLT_MIN__ 1.17549435e-38F
#define __VEC__ 10206
#define __CHAR_BIT__ 8
#define __WCHAR_MAX__ 2147483647
#define __DBL_DENORM_MIN__ 4.9406564584124654e-324
#define __ALTIVEC__ 1
#define __FLT_EVAL_METHOD__ 0
#define __DBL_MIN_10_EXP__ (-307)
#define __FINITE_MATH_ONLY__ 0
#define __GNUC_PATCHLEVEL__ 1
#define __SHRT_MAX__ 32767
#define __LDBL_MAX__ 1.79769313486231580793728971405301e+308L
#define __APPLE_CC__ 5493
#define __LDBL_MAX_EXP__ 1024
#define __SCHAR_MAX__ 127
#define __USER_LABEL_PREFIX__ _
#define __STDC_HOSTED__ 1
#define __LDBL_HAS_INFINITY__ 1
#define __DBL_DIG__ 15
#define __FLT_EPSILON__ 1.19209290e-7F
#define __LDBL_MIN__ 2.00416836000897277799610805135016e-292L
#define __ppc__ 1
#define __strong
#define __APPLE__ 1
#define __DECIMAL_DIG__ 33
#define __LDBL_HAS_QUIET_NAN__ 1
#define __DYNAMIC__ 1
#define __GNUC__ 4
#define __DBL_MAX__ 1.7976931348623157e+308
#define __DBL_HAS_INFINITY__ 1
#define OBJC_NEW_PROPERTIES 1
#define __weak
#define __DBL_MAX_EXP__ 1024
#define __LONG_LONG_MAX__ 9223372036854775807LL
#define __GXX_ABI_VERSION 1002
#define __FLT_MIN_EXP__ (-125)
#define __DBL_MIN__ 2.2250738585072014e-308
#define __DBL_HAS_QUIET_NAN__ 1
#define __pixel __attribute__((altivec(pixel__))) unsigned short
#define __REGISTER_PREFIX__
#define __NO_INLINE__ 1
#define _ARCH_PPC 1
#define __FLT_MANT_DIG__ 24
#define __VERSION__ "4.0.1 (Apple Inc. build 5493)"
#define __BIG_ENDIAN__ 1
#define __UINTMAX_TYPE__ long long unsigned int
#define __ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__ 1058
#define __SIZE_TYPE__ long unsigned int
#define __FLT_RADIX__ 2
#define __LDBL_EPSILON__ 4.94065645841246544176568792868221e-324L
#define __NATURAL_ALIGNMENT__ 1
#define __vector __attribute__((altivec(vector__)))
#define __FLT_HAS_QUIET_NAN__ 1
#define __bool __attribute__((altivec(bool__))) unsigned
#define __FLT_MAX_10_EXP__ 38
#define __LONG_MAX__ 2147483647L
#define __FLT_HAS_INFINITY__ 1
#define _BIG_ENDIAN 1
#define __LDBL_MANT_DIG__ 106
#define __CONSTANT_CFSTRINGS__ 1
#define __WCHAR_TYPE__ int
#define __FLT_DIG__ 6
#define __INT_MAX__ 2147483647
#define __LONG_DOUBLE_128__ 1
#define __FLT_MAX_EXP__ 128
#define __DBL_MANT_DIG__ 53
#define __WINT_TYPE__ int
#define __LDBL_MIN_EXP__ (-968)
#define __MACH__ 1
#define __LDBL_MAX_10_EXP__ 308
#define __DBL_EPSILON__ 2.2204460492503131e-16
#define __INTMAX_MAX__ 9223372036854775807LL
#define __FLT_DENORM_MIN__ 1.40129846e-45F
#define __PIC__ 1
#define __FLT_MAX__ 3.40282347e+38F
#define __FLT_MIN_10_EXP__ (-37)
#define __INTMAX_TYPE__ long long int
#define __GNUC_MINOR__ 0
#define __DBL_MAX_10_EXP__ 308
#define __LDBL_DENORM_MIN__ 4.94065645841246544176568792868221e-324L
#define __PTRDIFF_TYPE__ int
#define __LDBL_MIN_10_EXP__ (-291)
#define __LDBL_DIG__ 31
#define __POWERPC__ 1
#define __GNUC_GNU_INLINE__ 1

$ gcc-4.2 --version
powerpc-apple-darwin9-gcc-4.2.1 (GCC) 4.2.1 (Apple Inc. build 5577)
Copyright (C) 2007 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
$ gcc-4.2 -E -dM -x c /dev/null
#define __DBL_MIN_EXP__ (-1021)
#define __FLT_MIN__ 1.17549435e-38F
#define __DEC64_DEN__ 0.000000000000001E-383DD
#define __VEC__ 10206
#define __CHAR_BIT__ 8
#define _ARCH_PPCGR 1
#define __WCHAR_MAX__ 2147483647
#define __DBL_DENORM_MIN__ 4.9406564584124654e-324
#define __ALTIVEC__ 1
#define __FLT_EVAL_METHOD__ 0
#define __FINITE_MATH_ONLY__ 0
#define __GNUC_PATCHLEVEL__ 1
#define __DEC64_MAX_EXP__ 384
#define __SHRT_MAX__ 32767
#define __LDBL_MAX__ 1.79769313486231580793728971405301e+308L
#define __APPLE_CC__ 5577
#define __DEC32_EPSILON__ 1E-6DF
#define __LDBL_MAX_EXP__ 1024
#define __SCHAR_MAX__ 127
#define __USER_LABEL_PREFIX__ _
#define __LDBL_HAS_INFINITY__ 1
#define __DEC64_MIN_EXP__ (-383)
#define __DBL_DIG__ 15
#define __FLT_EPSILON__ 1.19209290e-7F
#define __LDBL_MIN__ 2.00416836000897277799610805135016e-292L
#define __DEC32_MAX__ 9.999999E96DF
#define __ppc__ 1
#define __strong
#define __APPLE__ 1
#define __DECIMAL_DIG__ 33
#define __LDBL_HAS_QUIET_NAN__ 1
#define __DYNAMIC__ 1
#define __GNUC__ 4
#define __FLT_HAS_DENORM__ 1
#define __DBL_MAX__ 1.7976931348623157e+308
#define __DBL_HAS_INFINITY__ 1
#define __DEC32_MIN_EXP__ (-95)
#define OBJC_NEW_PROPERTIES 1
#define __LDBL_HAS_DENORM__ 1
#define __DEC128_MAX__ 9.999999999999999999999999999999999E6144DL
#define __DEC32_MIN__ 1E-95DF
#define __weak
#define __DBL_MAX_EXP__ 1024
#define __DEC128_EPSILON__ 1E-33DL
#define __LONG_LONG_MAX__ 9223372036854775807LL
#define __GXX_ABI_VERSION 1002
#define __FLT_MIN_EXP__ (-125)
#define __DBL_MIN__ 2.2250738585072014e-308
#define __DBL_HAS_QUIET_NAN__ 1
#define __pixel __attribute__((altivec(pixel__))) unsigned short
#define __REGISTER_PREFIX__
#define __DBL_HAS_DENORM__ 1
#define __NO_INLINE__ 1
#define __DEC_EVAL_METHOD__ 2
#define _ARCH_PPC 1
#define __FLT_MANT_DIG__ 24
#define __VERSION__ "4.2.1 (Apple Inc. build 5577)"
#define __BIG_ENDIAN__ 1
#define __UINTMAX_TYPE__ long long unsigned int
#define __DEC128_MIN__ 1E-6143DL
#define __DEC64_EPSILON__ 1E-15DD
#define __DEC128_MIN_EXP__ (-6143)
#define __ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__ 1058
#define __SIZE_TYPE__ long unsigned int
#define __DEC32_DEN__ 0.000001E-95DF
#define __FLT_RADIX__ 2
#define __LDBL_EPSILON__ 4.94065645841246544176568792868221e-324L
#define __NATURAL_ALIGNMENT__ 1
#define __DBL_MIN_10_EXP__ (-307)
#define __LDBL_DIG__ 31
#define __vector __attribute__((altivec(vector__)))
#define __FLT_HAS_QUIET_NAN__ 1
#define __bool __attribute__((altivec(bool__))) unsigned
#define __FLT_MAX_10_EXP__ 38
#define __LONG_MAX__ 2147483647L
#define __FLT_HAS_INFINITY__ 1
#define __DEC64_MAX__ 9.999999999999999E384DD
#define __DEC64_MANT_DIG__ 16
#define __DEC32_MAX_EXP__ 96
#define _BIG_ENDIAN 1
#define __DEC128_DEN__ 0.000000000000000000000000000000001E-6143DL
#define __LDBL_MANT_DIG__ 106
#define __CONSTANT_CFSTRINGS__ 1
#define __WCHAR_TYPE__ int
#define __pic__ 2
#define __FLT_DIG__ 6
#define __INT_MAX__ 2147483647
#define __LONG_DOUBLE_128__ 1
#define __FLT_MAX_EXP__ 128
#define __DBL_MANT_DIG__ 53
#define __DEC64_MIN__ 1E-383DD
#define __WINT_TYPE__ int
#define __STDC_HOSTED__ 1
#define __LDBL_MIN_EXP__ (-968)
#define __MACH__ 1
#define __LDBL_MAX_10_EXP__ 308
#define __DBL_EPSILON__ 2.2204460492503131e-16
#define __INTMAX_MAX__ 9223372036854775807LL
#define __FLT_DENORM_MIN__ 1.40129846e-45F
#define __PIC__ 2
#define __FLT_MAX__ 3.40282347e+38F
#define __FLT_MIN_10_EXP__ (-37)
#define __INTMAX_TYPE__ long long int
#define __DEC128_MAX_EXP__ 6144
#define __GNUC_MINOR__ 2
#define __DEC32_MANT_DIG__ 7
#define __DBL_MAX_10_EXP__ 308
#define __LDBL_DENORM_MIN__ 4.94065645841246544176568792868221e-324L
#define __STDC__ 1
#define __PTRDIFF_TYPE__ int
#define __DEC128_MANT_DIG__ 34
#define __LDBL_MIN_10_EXP__ (-291)
#define __POWERPC__ 1
#define __GNUC_GNU_INLINE__ 1

/-F

[Qemu-devel] [PATCH for-1.1 3/3] tcg/ppc: Fix CONFIG_TCG_PASS_AREG0 mode

[Andreas Färber]

Adjust the tcg_out_qemu_{ld,st}() slow paths to pass AREG0 in r3.
Automate the register numbering to avoid double-coding the two modes,
and introduce TCG_TARGET_CALL_ALIGN_I64_ARG() macro to align for SVR4
but not for Darwin ABI.

Based on patch by malc.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 tcg/ppc/tcg-target.c |   48 +++++++++++++++++++++++++++++++++---------------
 1 files changed, 33 insertions(+), 15 deletions(-)

diff --git a/tcg/ppc/tcg-target.c b/tcg/ppc/tcg-target.c
index 5a651ce..ace5548 100644
--- a/tcg/ppc/tcg-target.c
+++ b/tcg/ppc/tcg-target.c
@@ -39,6 +39,16 @@ static uint8_t *tb_ret_addr;
 #define LR_OFFSET 4
 #endif
 
+#ifdef TCG_TARGET_CALL_ALIGN_ARGS
+#define TCG_TARGET_CALL_ALIGN_I64_ARG(reg) do { \
+        if (((reg) % 2) == 0) { \
+            (reg)++; \
+        } \
+    } while (0)
+#else
+#define TCG_TARGET_CALL_ALIGN_I64_ARG(reg) do { } while (0)
+#endif
+
 #define FAST_PATH
 
 #ifndef GUEST_BASE
@@ -513,7 +523,6 @@ static void tcg_out_call (TCGContext *s, tcg_target_long arg, int const_arg)
 #include "../../softmmu_defs.h"
 
 #ifdef CONFIG_TCG_PASS_AREG0
-#error CONFIG_TCG_PASS_AREG0 is not supported
 /* helper signature: helper_ld_mmu(CPUState *env, target_ulong addr,
    int mmu_idx) */
 static const void * const qemu_ld_helpers[4] = {
@@ -556,7 +565,7 @@ static void tcg_out_qemu_ld (TCGContext *s, const TCGArg *args, int opc)
 {
     int addr_reg, data_reg, data_reg2, r0, r1, rbase, bswap;
 #ifdef CONFIG_SOFTMMU
-    int mem_index, s_bits, r2;
+    int mem_index, s_bits, r2, ir;
     void *label1_ptr, *label2_ptr;
 #if TARGET_LONG_BITS == 64
     int addr_reg2;
@@ -618,14 +627,20 @@ static void tcg_out_qemu_ld (TCGContext *s, const TCGArg *args, int opc)
 #endif
 
     /* slow path */
+#ifdef CONFIG_TCG_PASS_AREG0
+    tcg_out_mov (s, TCG_TYPE_I32, 3, TCG_AREG0);
+    ir = 4;
+#else
+    ir = 3;
+#endif
 #if TARGET_LONG_BITS == 32
-    tcg_out_mov (s, TCG_TYPE_I32, 3, addr_reg);
-    tcg_out_movi (s, TCG_TYPE_I32, 4, mem_index);
+    tcg_out_mov (s, TCG_TYPE_I32, ir++, addr_reg);
 #else
-    tcg_out_mov (s, TCG_TYPE_I32, 3, addr_reg2);
-    tcg_out_mov (s, TCG_TYPE_I32, 4, addr_reg);
-    tcg_out_movi (s, TCG_TYPE_I32, 5, mem_index);
+    TCG_TARGET_CALL_ALIGN_I64_ARG (ir);
+    tcg_out_mov (s, TCG_TYPE_I32, ir++, addr_reg2);
+    tcg_out_mov (s, TCG_TYPE_I32, ir++, addr_reg);
 #endif
+    tcg_out_movi (s, TCG_TYPE_I32, ir, mem_index);
 
     tcg_out_call (s, (tcg_target_long) qemu_ld_helpers[s_bits], 1);
     switch (opc) {
@@ -814,13 +829,18 @@ static void tcg_out_qemu_st (TCGContext *s, const TCGArg *args, int opc)
 #endif
 
     /* slow path */
-#if TARGET_LONG_BITS == 32
-    tcg_out_mov (s, TCG_TYPE_I32, 3, addr_reg);
+#ifdef CONFIG_TCG_PASS_AREG0
+    tcg_out_mov (s, TCG_TYPE_I32, 3, TCG_AREG0);
     ir = 4;
 #else
-    tcg_out_mov (s, TCG_TYPE_I32, 3, addr_reg2);
-    tcg_out_mov (s, TCG_TYPE_I32, 4, addr_reg);
-    ir = 5;
+    ir = 3;
+#endif
+#if TARGET_LONG_BITS == 32
+    tcg_out_mov (s, TCG_TYPE_I32, ir++, addr_reg);
+#else
+    TCG_TARGET_CALL_ALIGN_I64_ARG (ir);
+    tcg_out_mov (s, TCG_TYPE_I32, ir++, addr_reg2);
+    tcg_out_mov (s, TCG_TYPE_I32, ir++, addr_reg);
 #endif
 
     switch (opc) {
@@ -844,9 +864,7 @@ static void tcg_out_qemu_st (TCGContext *s, const TCGArg *args, int opc)
         tcg_out_mov (s, TCG_TYPE_I32, ir, data_reg);
         break;
     case 3:
-#ifdef TCG_TARGET_CALL_ALIGN_ARGS
-        ir = 5;
-#endif
+        TCG_TARGET_CALL_ALIGN_I64_ARG (ir);
         tcg_out_mov (s, TCG_TYPE_I32, ir++, data_reg2);
         tcg_out_mov (s, TCG_TYPE_I32, ir, data_reg);
         break;
-- 
1.7.7

Re: [Qemu-devel] [Qemu-ppc] [PATCH for-1.1 3/3] tcg/ppc: Fix CONFIG_TCG_PASS_AREG0 mode

[Alexander Graf]

On 07.05.2012, at 01:46, Andreas Färber wrote:

> Adjust the tcg_out_qemu_{ld,st}() slow paths to pass AREG0 in r3.
> Automate the register numbering to avoid double-coding the two modes,
> and introduce TCG_TARGET_CALL_ALIGN_I64_ARG() macro to align for SVR4
> but not for Darwin ABI.
> 
> Based on patch by malc.

AREG0-free PPC works for me with this patch on a ppc32 host.

Tested-by: Alexander Graf <agraf@suse.de>


Alex

Re: [Qemu-devel] [Qemu-ppc] [PATCH for-1.1 3/3] tcg/ppc: Fix CONFIG_TCG_PASS_AREG0 mode

[Alexander Graf]

On 08.05.2012, at 19:39, Alexander Graf wrote:

> 
> On 07.05.2012, at 01:46, Andreas Färber wrote:
> 
>> Adjust the tcg_out_qemu_{ld,st}() slow paths to pass AREG0 in r3.
>> Automate the register numbering to avoid double-coding the two modes,
>> and introduce TCG_TARGET_CALL_ALIGN_I64_ARG() macro to align for SVR4
>> but not for Darwin ABI.
>> 
>> Based on patch by malc.
> 
> AREG0-free PPC works for me with this patch on a ppc32 host.
> 
> Tested-by: Alexander Graf <agraf@suse.de>

I take that one back - it breaks once things get more complex. Debugging ...


Alex

Re: [Qemu-devel] [Qemu-ppc] [PATCH for-1.1 3/3] tcg/ppc: Fix CONFIG_TCG_PASS_AREG0 mode

[Alexander Graf]

On 08.05.2012, at 19:43, Alexander Graf wrote:

> 
> On 08.05.2012, at 19:39, Alexander Graf wrote:
> 
>> 
>> On 07.05.2012, at 01:46, Andreas Färber wrote:
>> 
>>> Adjust the tcg_out_qemu_{ld,st}() slow paths to pass AREG0 in r3.
>>> Automate the register numbering to avoid double-coding the two modes,
>>> and introduce TCG_TARGET_CALL_ALIGN_I64_ARG() macro to align for SVR4
>>> but not for Darwin ABI.
>>> 
>>> Based on patch by malc.
>> 
>> AREG0-free PPC works for me with this patch on a ppc32 host.
>> 
>> Tested-by: Alexander Graf <agraf@suse.de>
> 
> I take that one back - it breaks once things get more complex. Debugging ...

I have no idea how this code could have ever worked. We are getting unknown register numbers as input variables. Then mr them into our C ABI parameter registers (r3+). Then we call the C helper to do the load/store for us.

Now, what if one of those input parameters is within r3-r7 (which is the highest register passed into the C ld function)? We'd happily do something like

  mr r3, r5
  mr r4, r3
  mr r5, ...

at which point we have long overwritten the actual value of r3!

The following patch on top of Andreas' patch makes ppc32 tcg work for me. I'd suggest committing his patch + the one below to have a working rc1 and take it from there.


Alex


diff --git a/tcg/ppc/tcg-target.c b/tcg/ppc/tcg-target.c
index ace5548..917bc39 100644
--- a/tcg/ppc/tcg-target.c
+++ b/tcg/ppc/tcg-target.c
@@ -116,11 +116,13 @@ static const int tcg_target_reg_alloc_order[] = {
 #ifdef _CALL_DARWIN
     TCG_REG_R2,
 #endif
+#if 0
     TCG_REG_R3,
     TCG_REG_R4,
     TCG_REG_R5,
     TCG_REG_R6,
     TCG_REG_R7,
+#endif
     TCG_REG_R8,
     TCG_REG_R9,
     TCG_REG_R10,

Re: [Qemu-devel] [Qemu-ppc] [PATCH for-1.1 3/3] tcg/ppc: Fix CONFIG_TCG_PASS_AREG0 mode

[Alexander Graf]

On 08.05.2012, at 20:20, Alexander Graf wrote:

> 
> On 08.05.2012, at 19:43, Alexander Graf wrote:
> 
>> 
>> On 08.05.2012, at 19:39, Alexander Graf wrote:
>> 
>>> 
>>> On 07.05.2012, at 01:46, Andreas Färber wrote:
>>> 
>>>> Adjust the tcg_out_qemu_{ld,st}() slow paths to pass AREG0 in r3.
>>>> Automate the register numbering to avoid double-coding the two modes,
>>>> and introduce TCG_TARGET_CALL_ALIGN_I64_ARG() macro to align for SVR4
>>>> but not for Darwin ABI.
>>>> 
>>>> Based on patch by malc.
>>> 
>>> AREG0-free PPC works for me with this patch on a ppc32 host.
>>> 
>>> Tested-by: Alexander Graf <agraf@suse.de>
>> 
>> I take that one back - it breaks once things get more complex. Debugging ...
> 
> I have no idea how this code could have ever worked. We are getting unknown register numbers as input variables. Then mr them into our C ABI parameter registers (r3+). Then we call the C helper to do the load/store for us.
> 
> Now, what if one of those input parameters is within r3-r7 (which is the highest register passed into the C ld function)? We'd happily do something like
> 
>  mr r3, r5
>  mr r4, r3
>  mr r5, ...
> 
> at which point we have long overwritten the actual value of r3!

This actually makes me wonder. How do we guarantee that all the volatile registers at the state of a ld/st are declared as clobbered?

[a few minutes later]

Aha! Through constraints. Then the below patch (which also fixes the issue for me) is a lot better of course:


diff --git a/tcg/ppc/tcg-target.c b/tcg/ppc/tcg-target.c
index ace5548..5750a2b 100644
--- a/tcg/ppc/tcg-target.c
+++ b/tcg/ppc/tcg-target.c
@@ -258,6 +258,9 @@ static int target_parse_constraint(TCGArgConstraint *ct, const char **pct_str)
         tcg_regset_set32(ct->u.regs, 0, 0xffffffff);
         tcg_regset_reset_reg(ct->u.regs, TCG_REG_R3);
         tcg_regset_reset_reg(ct->u.regs, TCG_REG_R4);
+        tcg_regset_reset_reg(ct->u.regs, TCG_REG_R5);
+        tcg_regset_reset_reg(ct->u.regs, TCG_REG_R6);
+        tcg_regset_reset_reg(ct->u.regs, TCG_REG_R7);
         break;
     case 'K':                   /* qemu_st[8..32] constraint */
         ct->ct |= TCG_CT_REG;
@@ -267,6 +270,7 @@ static int target_parse_constraint(TCGArgConstraint *ct, const char **pct_str)
         tcg_regset_reset_reg(ct->u.regs, TCG_REG_R5);
 #if TARGET_LONG_BITS == 64
         tcg_regset_reset_reg(ct->u.regs, TCG_REG_R6);
+        tcg_regset_reset_reg(ct->u.regs, TCG_REG_R7);
 #endif
         break;
     case 'M':                   /* qemu_st64 constraint */

Re: [Qemu-devel] [Qemu-ppc] [PATCH for-1.1 3/3] tcg/ppc: Fix CONFIG_TCG_PASS_AREG0 mode

[Andreas Färber]

Am 08.05.2012 19:43, schrieb Alexander Graf:
> 
> On 08.05.2012, at 19:39, Alexander Graf wrote:
> 
>> On 07.05.2012, at 01:46, Andreas Färber wrote:
>>
>>> Adjust the tcg_out_qemu_{ld,st}() slow paths to pass AREG0 in r3.
>>> Automate the register numbering to avoid double-coding the two modes,
>>> and introduce TCG_TARGET_CALL_ALIGN_I64_ARG() macro to align for SVR4
>>> but not for Darwin ABI.
>>>
>>> Based on patch by malc.
>>
>> AREG0-free PPC works for me with this patch on a ppc32 host.
>>
>> Tested-by: Alexander Graf <agraf@suse.de>
> 
> I take that one back - it breaks once things get more complex. Debugging ...

We've been modifying the slow path here - I wonder if any changes are
needed for the fast path? Don't understand that one unfortunately...

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] [Qemu-ppc] [PATCH for-1.1 3/3] tcg/ppc: Fix CONFIG_TCG_PASS_AREG0 mode

[Alexander Graf]

On 08.05.2012, at 21:34, Andreas Färber wrote:

> Am 08.05.2012 19:43, schrieb Alexander Graf:
>> 
>> On 08.05.2012, at 19:39, Alexander Graf wrote:
>> 
>>> On 07.05.2012, at 01:46, Andreas Färber wrote:
>>> 
>>>> Adjust the tcg_out_qemu_{ld,st}() slow paths to pass AREG0 in r3.
>>>> Automate the register numbering to avoid double-coding the two modes,
>>>> and introduce TCG_TARGET_CALL_ALIGN_I64_ARG() macro to align for SVR4
>>>> but not for Darwin ABI.
>>>> 
>>>> Based on patch by malc.
>>> 
>>> AREG0-free PPC works for me with this patch on a ppc32 host.
>>> 
>>> Tested-by: Alexander Graf <agraf@suse.de>
>> 
>> I take that one back - it breaks once things get more complex. Debugging ...
> 
> We've been modifying the slow path here - I wonder if any changes are
> needed for the fast path? Don't understand that one unfortunately...

I already posted the fix :)


Alex

Re: [Qemu-devel] [Qemu-ppc] [PATCH for-1.1 0/3] tcg/ppc: AREG0 support and Darwin fixes

[Alexander Graf]

On 07.05.2012, at 01:46, Andreas Färber wrote:

> Hello malc,
> 
> This series fixes two long-standing issues on Darwin/ppc and, based on your
> second patch, fixes the AREG0 mode for Linux and Darwin and thus the ppc build.
> 
> Compared to your last patch I have tried to avoid the { int ir; } block, and
> I've combined both modes in one code path using ir and a new macro to handle
> the alignment differences in a more explicit way.
> 
> Tested on Linux and Darwin, using i386 and x86_64 (non-AREG0) as well as sparc
> and sparc64 (AREG0). On Darwin I observed a reproducible hang in fsck during
> INIT under Debian/sparc, which given the lateness and now unified code paths
> (i.e., Haiku/i386 and HelenOS/sparc64 working) I'd attribute to lingering
> main loop / signal handling issues.
> 
> Regards,
> Andreas
> 
> Cc: malc <av1474@comtv.ru>

Malc, ping? :)


Alex

Re: [Qemu-devel] [Qemu-ppc] [PATCH for-1.1 0/3] tcg/ppc: AREG0 support and Darwin fixes

[malc]

On Tue, 8 May 2012, Alexander Graf wrote:

> 
> On 07.05.2012, at 01:46, Andreas F?rber wrote:
> 
> > Hello malc,
> > 
> > This series fixes two long-standing issues on Darwin/ppc and, based on your
> > second patch, fixes the AREG0 mode for Linux and Darwin and thus the ppc build.
> > 
> > Compared to your last patch I have tried to avoid the { int ir; } block, and
> > I've combined both modes in one code path using ir and a new macro to handle
> > the alignment differences in a more explicit way.
> > 
> > Tested on Linux and Darwin, using i386 and x86_64 (non-AREG0) as well as sparc
> > and sparc64 (AREG0). On Darwin I observed a reproducible hang in fsck during
> > INIT under Debian/sparc, which given the lateness and now unified code paths
> > (i.e., Haiku/i386 and HelenOS/sparc64 working) I'd attribute to lingering
> > main loop / signal handling issues.
> > 
> > Regards,
> > Andreas
> > 
> > Cc: malc <av1474@comtv.ru>
> 
> Malc, ping? :)

I raised my minor objections to Andreas on IRC, they are not yet
addressed.

-- 
mailto:av1474@comtv.ru

Re: [Qemu-devel] [Qemu-ppc] [PATCH for-1.1 0/3] tcg/ppc: AREG0 support and Darwin fixes

[Alexander Graf]

On 08.05.2012, at 19:39, malc wrote:

> On Tue, 8 May 2012, Alexander Graf wrote:
> 
>> 
>> On 07.05.2012, at 01:46, Andreas F?rber wrote:
>> 
>>> Hello malc,
>>> 
>>> This series fixes two long-standing issues on Darwin/ppc and, based on your
>>> second patch, fixes the AREG0 mode for Linux and Darwin and thus the ppc build.
>>> 
>>> Compared to your last patch I have tried to avoid the { int ir; } block, and
>>> I've combined both modes in one code path using ir and a new macro to handle
>>> the alignment differences in a more explicit way.
>>> 
>>> Tested on Linux and Darwin, using i386 and x86_64 (non-AREG0) as well as sparc
>>> and sparc64 (AREG0). On Darwin I observed a reproducible hang in fsck during
>>> INIT under Debian/sparc, which given the lateness and now unified code paths
>>> (i.e., Haiku/i386 and HelenOS/sparc64 working) I'd attribute to lingering
>>> main loop / signal handling issues.
>>> 
>>> Regards,
>>> Andreas
>>> 
>>> Cc: malc <av1474@comtv.ru>
>> 
>> Malc, ping? :)
> 
> I raised my minor objections to Andreas on IRC, they are not yet
> addressed.

Like? I'd like to have a working rc1.


Alex

Re: [Qemu-devel] [Qemu-ppc] [PATCH for-1.1 0/3] tcg/ppc: AREG0 support and Darwin fixes

[Andreas Färber]

Am 08.05.2012 20:09, schrieb Alexander Graf:
> 
> On 08.05.2012, at 19:39, malc wrote:
> 
>> On Tue, 8 May 2012, Alexander Graf wrote:
>>
>>> On 07.05.2012, at 01:46, Andreas F?rber wrote:
>>>
>>>> This series fixes two long-standing issues on Darwin/ppc and, based on your
>>>> second patch, fixes the AREG0 mode for Linux and Darwin and thus the ppc build.
>>>>
>>>> Compared to your last patch I have tried to avoid the { int ir; } block, and
>>>> I've combined both modes in one code path using ir and a new macro to handle
>>>> the alignment differences in a more explicit way.
>>>>
>>>> Tested on Linux and Darwin, using i386 and x86_64 (non-AREG0) as well as sparc
>>>> and sparc64 (AREG0). On Darwin I observed a reproducible hang in fsck during
>>>> INIT under Debian/sparc, which given the lateness and now unified code paths
>>>> (i.e., Haiku/i386 and HelenOS/sparc64 working) I'd attribute to lingering
>>>> main loop / signal handling issues.

What I was referring to here btw is that I've in one case seen Haiku
continue shutting down once I grabbed and moved the mouse in Cocoa. Not
observed on Linux with SDL or VNC.

>>> Malc, ping? :)
>>
>> I raised my minor objections to Andreas on IRC, they are not yet
>> addressed.
> 
> Like? I'd like to have a working rc1.

He didn't like my _CALL_DARWIN fix in patch 2/3 (which is totally
independent of 3/3).

On patch 3/3 he didn't like my alignment macro. I don't have a better
one though, suggestions or patches welcome. Ideal might be some
ROUND_TO_ODD() macro, but the problem is that for Darwin/AIX where it's
no-op it shouldn't result in a "statement without effect" warning.
Therefore my do { } while (0) as opposed to ir = MACRO(ir).

I do think though that any half-baked patch that doesn't regress is
better than the current #error situation.

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] [Qemu-ppc] [PATCH for-1.1 0/3] tcg/ppc: AREG0 support and Darwin fixes

[Alexander Graf]

On 08.05.2012, at 21:29, Andreas Färber wrote:

> Am 08.05.2012 20:09, schrieb Alexander Graf:
>> 
>> On 08.05.2012, at 19:39, malc wrote:
>> 
>>> On Tue, 8 May 2012, Alexander Graf wrote:
>>> 
>>>> On 07.05.2012, at 01:46, Andreas F?rber wrote:
>>>> 
>>>>> This series fixes two long-standing issues on Darwin/ppc and, based on your
>>>>> second patch, fixes the AREG0 mode for Linux and Darwin and thus the ppc build.
>>>>> 
>>>>> Compared to your last patch I have tried to avoid the { int ir; } block, and
>>>>> I've combined both modes in one code path using ir and a new macro to handle
>>>>> the alignment differences in a more explicit way.
>>>>> 
>>>>> Tested on Linux and Darwin, using i386 and x86_64 (non-AREG0) as well as sparc
>>>>> and sparc64 (AREG0). On Darwin I observed a reproducible hang in fsck during
>>>>> INIT under Debian/sparc, which given the lateness and now unified code paths
>>>>> (i.e., Haiku/i386 and HelenOS/sparc64 working) I'd attribute to lingering
>>>>> main loop / signal handling issues.
> 
> What I was referring to here btw is that I've in one case seen Haiku
> continue shutting down once I grabbed and moved the mouse in Cocoa. Not
> observed on Linux with SDL or VNC.
> 
>>>> Malc, ping? :)
>>> 
>>> I raised my minor objections to Andreas on IRC, they are not yet
>>> addressed.
>> 
>> Like? I'd like to have a working rc1.
> 
> He didn't like my _CALL_DARWIN fix in patch 2/3 (which is totally
> independent of 3/3).
> 
> On patch 3/3 he didn't like my alignment macro. I don't have a better
> one though, suggestions or patches welcome. Ideal might be some
> ROUND_TO_ODD() macro, but the problem is that for Darwin/AIX where it's
> no-op it shouldn't result in a "statement without effect" warning.
> Therefore my do { } while (0) as opposed to ir = MACRO(ir).

static inline int round_reg_i64(int input_reg)
{
#ifdef WHATEVER_CONDITION
    if (input_reg % 2) {
        reg++;
    }
#endif

    return reg;
}

[...]

  ir = round_reg_i64(ir);

> 
> I do think though that any half-baked patch that doesn't regress is
> better than the current #error situation.

Yes, please! Let's get what we have in for rc1 and take it from there.


Alex

Re: [Qemu-devel] [Qemu-ppc] [PATCH for-1.1 0/3] tcg/ppc: AREG0 support and Darwin fixes

[Andreas Färber]

Am 08.05.2012 21:42, schrieb Alexander Graf:
> 
> On 08.05.2012, at 21:29, Andreas Färber wrote:
> 
>> On patch 3/3 he didn't like my alignment macro. I don't have a better
>> one though, suggestions or patches welcome. Ideal might be some
>> ROUND_TO_ODD() macro, but the problem is that for Darwin/AIX where it's
>> no-op it shouldn't result in a "statement without effect" warning.
>> Therefore my do { } while (0) as opposed to ir = MACRO(ir).
> 
> static inline int round_reg_i64(int input_reg)
> {
> #ifdef WHATEVER_CONDITION
>     if (input_reg % 2) {
>         reg++;
>     }
> #endif
> 
>     return reg;
> }
> 
> [...]
> 
>   ir = round_reg_i64(ir);

I think he didn't like the mod check either but the suggestion
<malc> [...] reg += reg & TCG_TARGET_CALL_ALIGN_ARGS [...]
doesn't work well when TCG_TARGET_CALL_ALIGN_ARGS is undefined.
And defining it with value 0 seems unsafe to me.

What about the following? (untested)

static inline int tcg_target_call_align_i64_reg(int reg)
{
#ifdef TCG_TARGET_CALL_ALIGN_ARGS
    /* Round up to next odd register */
    return (reg & ~1) + 1;
#else
    return reg;
#endif
}

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] [Qemu-ppc] [PATCH for-1.1 0/3] tcg/ppc: AREG0 support and Darwin fixes

[malc]

On Tue, 8 May 2012, Andreas F?rber wrote:

> Am 08.05.2012 21:42, schrieb Alexander Graf:
> > 
> > On 08.05.2012, at 21:29, Andreas F?rber wrote:
> > 
> >> On patch 3/3 he didn't like my alignment macro. I don't have a better
> >> one though, suggestions or patches welcome. Ideal might be some
> >> ROUND_TO_ODD() macro, but the problem is that for Darwin/AIX where it's
> >> no-op it shouldn't result in a "statement without effect" warning.
> >> Therefore my do { } while (0) as opposed to ir = MACRO(ir).
> > 
> > static inline int round_reg_i64(int input_reg)
> > {
> > #ifdef WHATEVER_CONDITION
> >     if (input_reg % 2) {
> >         reg++;
> >     }
> > #endif
> > 
> >     return reg;
> > }
> > 
> > [...]
> > 
> >   ir = round_reg_i64(ir);
> 
> I think he didn't like the mod check either but the suggestion
> <malc> [...] reg += reg & TCG_TARGET_CALL_ALIGN_ARGS [...]
> doesn't work well when TCG_TARGET_CALL_ALIGN_ARGS is undefined.
> And defining it with value 0 seems unsafe to me.
> 
> What about the following? (untested)

I'd much rather have the ifdefery scattered around the code than
having to remember what this function does, but that's just me,
not hard bent on it..

-- 
mailto:av1474@comtv.ru

Re: [Qemu-devel] [Qemu-ppc] [PATCH for-1.1 0/3] tcg/ppc: AREG0 support and Darwin fixes

[Andreas Färber]

Am 08.05.2012 22:28, schrieb malc:
> On Tue, 8 May 2012, Andreas F?rber wrote:
> 
>> What about the following? (untested)
> 
> I'd much rather have the ifdefery scattered around the code than
> having to remember what this function does,

The name was supposed to explain that. :)

> but that's just me,
> not hard bent on it..

Fine with me; we still need to agree on what to put inside the #ifdef
then. ;)

/-F

-- 
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] [Qemu-ppc] [PATCH for-1.1 0/3] tcg/ppc: AREG0 support and Darwin fixes

[malc]

On Tue, 8 May 2012, Andreas F?rber wrote:

> Am 08.05.2012 22:28, schrieb malc:
> > On Tue, 8 May 2012, Andreas F?rber wrote:
> > 
> >> What about the following? (untested)
> > 
> > I'd much rather have the ifdefery scattered around the code than
> > having to remember what this function does,
> 
> The name was supposed to explain that. :)
> 
> > but that's just me,
> > not hard bent on it..
> 
> Fine with me; we still need to agree on what to put inside the #ifdef
> then. ;)
> 

#ifdef MOO
reg = reg + 1 - (reg & 1);
#endif

-- 
mailto:av1474@comtv.ru

Re: [Qemu-devel] [Qemu-ppc] [PATCH for-1.1 0/3] tcg/ppc: AREG0 support and Darwin fixes

[malc]

On Wed, 9 May 2012, malc wrote:

> On Tue, 8 May 2012, Andreas F?rber wrote:
> 
> > Am 08.05.2012 22:28, schrieb malc:
> > > On Tue, 8 May 2012, Andreas F?rber wrote:
> > > 
> > >> What about the following? (untested)
> > > 
> > > I'd much rather have the ifdefery scattered around the code than
> > > having to remember what this function does,
> > 
> > The name was supposed to explain that. :)
> > 
> > > but that's just me,
> > > not hard bent on it..
> > 
> > Fine with me; we still need to agree on what to put inside the #ifdef
> > then. ;)
> > 
> 
> #ifdef MOO
> reg = reg + 1 - (reg & 1);
> #endif
> 

Atrocious... 

#ifdef MOO
reg |= 1;
#endif

-- 
mailto:av1474@comtv.ru

Re: [Qemu-devel] [Qemu-ppc] [PATCH for-1.1 0/3] tcg/ppc: AREG0 support and Darwin fixes

[Andreas Färber]

Am 08.05.2012 19:39, schrieb malc:
> On Tue, 8 May 2012, Alexander Graf wrote:
> 
>>
>> On 07.05.2012, at 01:46, Andreas F?rber wrote:
>>
>>> Hello malc,
>>>
>>> This series fixes two long-standing issues on Darwin/ppc and, based on your
>>> second patch, fixes the AREG0 mode for Linux and Darwin and thus the ppc build.
>>>
>>> Compared to your last patch I have tried to avoid the { int ir; } block, and
>>> I've combined both modes in one code path using ir and a new macro to handle
>>> the alignment differences in a more explicit way.
>>>
>>> Tested on Linux and Darwin, using i386 and x86_64 (non-AREG0) as well as sparc
>>> and sparc64 (AREG0). On Darwin I observed a reproducible hang in fsck during
>>> INIT under Debian/sparc, which given the lateness and now unified code paths
>>> (i.e., Haiku/i386 and HelenOS/sparc64 working) I'd attribute to lingering
>>> main loop / signal handling issues.
>>>
>>> Regards,
>>> Andreas
>>>
>>> Cc: malc <av1474@comtv.ru>
>>
>> Malc, ping? :)
> 
> I raised my minor objections to Andreas on IRC, they are not yet
> addressed.

Do you have any better suggestion based on the gcc output you requested?
Should I rather replace all #ifdef _CALL_DARWIN with #if defined
_CALL_DARWIN || defined __APPLE__, as seen in tcg-target.h?

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] [Qemu-ppc] [PATCH for-1.1 0/3] tcg/ppc: AREG0 support and Darwin fixes

[malc]

On Tue, 8 May 2012, Andreas F?rber wrote:

> Am 08.05.2012 19:39, schrieb malc:
> > On Tue, 8 May 2012, Alexander Graf wrote:
> > 
> >>
> >> On 07.05.2012, at 01:46, Andreas F?rber wrote:
> >>
> >>> Hello malc,
> >>>
> >>> This series fixes two long-standing issues on Darwin/ppc and, based on your
> >>> second patch, fixes the AREG0 mode for Linux and Darwin and thus the ppc build.
> >>>
> >>> Compared to your last patch I have tried to avoid the { int ir; } block, and
> >>> I've combined both modes in one code path using ir and a new macro to handle
> >>> the alignment differences in a more explicit way.
> >>>
> >>> Tested on Linux and Darwin, using i386 and x86_64 (non-AREG0) as well as sparc
> >>> and sparc64 (AREG0). On Darwin I observed a reproducible hang in fsck during
> >>> INIT under Debian/sparc, which given the lateness and now unified code paths
> >>> (i.e., Haiku/i386 and HelenOS/sparc64 working) I'd attribute to lingering
> >>> main loop / signal handling issues.
> >>>
> >>> Regards,
> >>> Andreas
> >>>
> >>> Cc: malc <av1474@comtv.ru>
> >>
> >> Malc, ping? :)
> > 
> > I raised my minor objections to Andreas on IRC, they are not yet
> > addressed.
> 
> Do you have any better suggestion based on the gcc output you requested?
> Should I rather replace all #ifdef _CALL_DARWIN with #if defined
> _CALL_DARWIN || defined __APPLE__, as seen in tcg-target.h?
> 

I'd like to understand what's going on and why, as for why i don't like
<under some condition> #define _CALL_DARWIN is that it defines something
with a reserved name, replacing all instances of _CALL_DARWIN with
<something> and using that instead is better.

-- 
mailto:av1474@comtv.ru