On 2011-05-25 14:41, Andrew Lutomirski wrote:
On Tue, May 24, 2011 at 10:04 AM, David Henningsson
<david.hennings...@canonical.com>  wrote:
Ok, so there is still a high-pitched noise with 5.1, although I tried to fix
that a while ago. I now know what the error is, but perhaps somewhat with
more GCC-ASM skills can help me fix it.

I spent parts of yesterday and today analysing it and came up with the
following conclusion:

Here's the asm code as written in svolume_sse.c, function
pa_volume_s16ne_sse2:

#define MOD_ADD(a,b) \
      " add "#a", %3   \n\t" /* channel += inc          */ \
      " mov %3, %4     \n\t"                                \
      " sub "#b", %4   \n\t" /* tmp = channel - channels */ \
      " cmovae %4, %3  \n\t" /* if (tmp>= 0) channel = tmp  */

And called with: MOD_ADD($8, %5)

Here's what it shows up with gdb (on i386) :

   0x00f3fbe6<+550>:   add    $0x8,%edi
   0x00f3fbe9<+553>:   mov    %edi,%ecx
   0x00f3fbeb<+555>:   sub    %edi,%ecx
   0x00f3fbed<+557>:   cmovae %ecx,%edi

The error: both %3 and %5 is turned into the %edi register.

Here's the register allocation:

: "+r" (samples), "+r" (volumes), "+r" (length), "=D" (channel), "=&r"
(temp)
: "rm" ((pa_reg_x86)channels)
: "cc"

As I can tell from the above the "=D" forces %3 to go into %edi, but I don't
know how to tell GCC not to use %edi for %5 (channels) as well, does anybody
know? Can this be a GCC bug?


Either use "+D" for channel or "&rm" for channels.

The manual here:

http://gcc.gnu.org/onlinedocs/gcc/Modifiers.html#Modifiers

is pretty good.

Thanks! Maybe I've found it if I had googled it better. Unfortunately that didn't work, but after several attempts I came up with something that seems to work here. The problem seems to be with the "rm" thing itself.

Colin (or someone else), if you have time, would you give this a test spin and apply if it works for you?

--
David Henningsson, Canonical Ltd.
http://launchpad.net/~diwic
>From 75d0e5f5ad7764eba560810c958c5dfa4a248f81 Mon Sep 17 00:00:00 2001
From: David Henningsson <david.hennings...@canonical.com>
Date: Fri, 27 May 2011 09:28:57 +0200
Subject: [PATCH] SSE/MMX: Fix problem with highpitched noise on i386

The "rm" basm constraint doesn't work with my version of gcc (4.5.2),
not even in a simple example. Since we usually only have 5 registers
available on i386, force it to be memory on that architecture.

Signed-off-by: David Henningsson <david.hennings...@canonical.com>
---
 src/pulsecore/svolume_mmx.c |   12 ++++++++++--
 src/pulsecore/svolume_sse.c |   12 ++++++++++--
 2 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/src/pulsecore/svolume_mmx.c b/src/pulsecore/svolume_mmx.c
index a71a39b..129d7f0 100644
--- a/src/pulsecore/svolume_mmx.c
+++ b/src/pulsecore/svolume_mmx.c
@@ -155,7 +155,11 @@ pa_volume_s16ne_mmx (int16_t *samples, int32_t *volumes, unsigned channels, unsi
         " emms                          \n\t"
 
         : "+r" (samples), "+r" (volumes), "+r" (length), "=D" ((pa_reg_x86)channel), "=&r" (temp)
-        : "rm" ((pa_reg_x86)channels)
+#if defined (__i386__)
+        : "m" ((pa_reg_x86)channels)
+#else
+        : "r" ((pa_reg_x86)channels)
+#endif
         : "cc"
     );
 }
@@ -232,7 +236,11 @@ pa_volume_s16re_mmx (int16_t *samples, int32_t *volumes, unsigned channels, unsi
         " emms                          \n\t"
 
         : "+r" (samples), "+r" (volumes), "+r" (length), "=D" ((pa_reg_x86)channel), "=&r" (temp)
-        : "rm" ((pa_reg_x86)channels)
+#if defined (__i386__)
+        : "m" ((pa_reg_x86)channels)
+#else
+        : "r" ((pa_reg_x86)channels)
+#endif
         : "cc"
     );
 }
diff --git a/src/pulsecore/svolume_sse.c b/src/pulsecore/svolume_sse.c
index 5983164..ccf45e7 100644
--- a/src/pulsecore/svolume_sse.c
+++ b/src/pulsecore/svolume_sse.c
@@ -154,7 +154,11 @@ pa_volume_s16ne_sse2 (int16_t *samples, int32_t *volumes, unsigned channels, uns
         "8:                             \n\t"
 
         : "+r" (samples), "+r" (volumes), "+r" (length), "=D" (channel), "=&r" (temp)
-        : "rm" ((pa_reg_x86)channels)
+#if defined (__i386__)
+        : "m" ((pa_reg_x86)channels)
+#else
+        : "r" ((pa_reg_x86)channels)
+#endif
         : "cc"
     );
 }
@@ -244,7 +248,11 @@ pa_volume_s16re_sse2 (int16_t *samples, int32_t *volumes, unsigned channels, uns
         "8:                             \n\t"
 
         : "+r" (samples), "+r" (volumes), "+r" (length), "=D" (channel), "=&r" (temp)
-        : "rm" ((pa_reg_x86)channels)
+#if defined (__i386__)
+        : "m" ((pa_reg_x86)channels)
+#else
+        : "r" ((pa_reg_x86)channels)
+#endif
         : "cc"
     );
 }
-- 
1.7.4.1

_______________________________________________
pulseaudio-discuss mailing list
pulseaudio-discuss@mail.0pointer.de
https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss

Reply via email to