[PATCH 0/2] two bug fixes for tremor and libatomics-ops

[PATCH 0/2] two bug fixes for tremor and libatomics-ops

[Jesse Zhang]

The following changes since commit 3dee534f1e25109e0bdb681de0746c336f4b8840:

  lib/oeqa: fix dependecy check (2013-07-16 10:04:17 +0100)

are available in the git repository at:

  git://git.yoctoproject.org/poky-contrib szhang/bugs
  http://git.yoctoproject.org/cgit.cgi/poky-contrib/log/?h=szhang/bugs

Jesse Zhang (2):
  tremor: add a patch to build with arm thumb
  libatomics-ops: force omit frame pointer for x86 builds

 .../pulseaudio/libatomics-ops_7.2.bb               |   4 +
 .../tremor/tremor-20120314/tremor-arm-thumb2.patch | 104 +++++++++++++++++++++
 meta/recipes-multimedia/tremor/tremor_20120314.bb  |   1 +
 3 files changed, 109 insertions(+)
 create mode 100644 meta/recipes-multimedia/tremor/tremor-20120314/tremor-arm-thumb2.patch

-- 
1.7.11.7

[PATCH 1/2] tremor: add a patch to build with arm thumb

[Jesse Zhang]

Add IT instructions so that it builds with thumb tunes. ARM mode won't
be affected since IT is a pseudo-instruction in ARM mode.

Signed-off-by: Sen Zhang <sen.zhang@windriver.com>
---
 .../tremor/tremor-20120314/tremor-arm-thumb2.patch | 104 +++++++++++++++++++++
 meta/recipes-multimedia/tremor/tremor_20120314.bb  |   1 +
 2 files changed, 105 insertions(+)
 create mode 100644 meta/recipes-multimedia/tremor/tremor-20120314/tremor-arm-thumb2.patch

diff --git a/meta/recipes-multimedia/tremor/tremor-20120314/tremor-arm-thumb2.patch b/meta/recipes-multimedia/tremor/tremor-20120314/tremor-arm-thumb2.patch
new file mode 100644
index 0000000..2049542
--- /dev/null
+++ b/meta/recipes-multimedia/tremor/tremor-20120314/tremor-arm-thumb2.patch
@@ -0,0 +1,104 @@
+From: Xin Ouyang <Xin.Ouyang@windriver.com>
+Date: Mon, 16 Jul 2012 13:29:34 +0800
+Subject: [PATCH] tremor: add IT instructions for arm thumb2 tune flags.
+
+Upstream-Status: Pending
+
+In Thumb-2, most instructions do not have a built in condition code (except for 
+conditional branches). Instead, short sequences of instructions which are to be 
+executed conditionally can be preceded by a special "IT instruction" which 
+describes the condition and which of the following instructions should be 
+executed if the condition is false respectively. 
+
+For the ARM/Thumb IT(If-Then) instruction:
+http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0489c/Cjabicci.html
+
+Signed-off-by: Xin Ouyang <Xin.Ouyang@windriver.com>
+---
+ asm_arm.h |   14 ++++++++++++++
+ 1 file changed, 14 insertions(+)
+
+diff --git a/asm_arm.h b/asm_arm.h
+index c3bda00..823c54f 100755
+--- a/asm_arm.h
++++ b/asm_arm.h
+@@ -108,9 +108,11 @@ static inline void XNPROD31(ogg_int32_t  a, ogg_int32_t  b,
+ static inline ogg_int32_t CLIP_TO_15(ogg_int32_t x) {
+   int tmp;
+   asm volatile("subs	%1, %0, #32768\n\t"
++	       "itt     pl\n\t"
+ 	       "movpl	%0, #0x7f00\n\t"
+ 	       "orrpl	%0, %0, #0xff\n"
+ 	       "adds	%1, %0, #32768\n\t"
++	       "it      mi\n\t"
+ 	       "movmi	%0, #0x8000"
+ 	       : "+r"(x),"=r"(tmp)
+ 	       :
+@@ -139,10 +141,12 @@ static inline void lsp_loop_asm(ogg_uint32_t *qip,ogg_uint32_t *pip,
+       
+       "ldmdb   r0!,{r1,r3};"
+       "subs    r1,r1,%4;"          //ilsp[j]-wi
++      "it      mi;"
+       "rsbmi   r1,r1,#0;"          //labs(ilsp[j]-wi)
+       "umull   %0,r2,r1,%0;"       //qi*=labs(ilsp[j]-wi)
+       
+       "subs    r1,r3,%4;"          //ilsp[j+1]-wi
++      "it      mi;"
+       "rsbmi   r1,r1,#0;"          //labs(ilsp[j+1]-wi)
+       "umull   %1,r3,r1,%1;"       //pi*=labs(ilsp[j+1]-wi)
+       
+@@ -167,6 +171,7 @@ static inline void lsp_loop_asm(ogg_uint32_t *qip,ogg_uint32_t *pip,
+       "mov     r0,#0x4000;\n"
+       
+       "subs    r1,r1,%4;\n"          //ilsp[j]-wi
++      "it      mi;\n"
+       "rsbmi   r1,r1,#0;\n"          //labs(ilsp[j]-wi)
+       "umull   %0,r2,r1,%0;\n"       //qi*=labs(ilsp[j]-wi)
+       "umull   %1,r3,r0,%1;\n"       //pi*=labs(ilsp[j+1]-wi)
+@@ -190,18 +195,23 @@ static inline void lsp_loop_asm(ogg_uint32_t *qip,ogg_uint32_t *pip,
+       "mov     r2,#0;"
+       "orr     r1,%0,%1;"
+       "tst     r1,#0xff000000;"
++      "itt     ne;"
+       "addne   r2,r2,#8;"
+       "movne   r1,r1,lsr #8;"
+       "tst     r1,#0x00f00000;"
++      "itt     ne;"
+       "addne   r2,r2,#4;"
+       "movne   r1,r1,lsr #4;"
+       "tst     r1,#0x000c0000;"
++      "itt     ne;"
+       "addne   r2,r2,#2;"
+       "movne   r1,r1,lsr #2;"
+       "tst     r1,#0x00020000;"
++      "itt     ne;"
+       "addne   r2,r2,#1;"
+       "movne   r1,r1,lsr #1;"
+       "tst     r1,#0x00010000;"
++      "it      ne;"
+       "addne   r2,r2,#1;"
+       "mov     %0,%0,lsr r2;"
+       "mov     %1,%1,lsr r2;"
+@@ -222,15 +232,19 @@ static inline void lsp_norm_asm(ogg_uint32_t *qip,ogg_int32_t *qexpp){
+   ogg_int32_t qexp=*qexpp;
+ 
+   asm("tst     %0,#0x0000ff00;"
++      "itt     eq;"
+       "moveq   %0,%0,lsl #8;"
+       "subeq   %1,%1,#8;"
+       "tst     %0,#0x0000f000;"
++      "itt     eq;"
+       "moveq   %0,%0,lsl #4;"
+       "subeq   %1,%1,#4;"
+       "tst     %0,#0x0000c000;"
++      "itt     eq;"
+       "moveq   %0,%0,lsl #2;"
+       "subeq   %1,%1,#2;"
+       "tst     %0,#0x00008000;"
++      "itt     eq;"
+       "moveq   %0,%0,lsl #1;"
+       "subeq   %1,%1,#1;"
+       : "+r"(qi),"+r"(qexp)
+-- 
+1.7.9.5
+
diff --git a/meta/recipes-multimedia/tremor/tremor_20120314.bb b/meta/recipes-multimedia/tremor/tremor_20120314.bb
index a11e799..d95fdf1 100644
--- a/meta/recipes-multimedia/tremor/tremor_20120314.bb
+++ b/meta/recipes-multimedia/tremor/tremor_20120314.bb
@@ -10,6 +10,7 @@ PR = "r1"
 
 SRC_URI = "svn://svn.xiph.org/trunk;module=Tremor;rev=18221;protocol=http \
            file://obsolete_automake_macros.patch;striplevel=0 \
+           file://tremor-arm-thumb2.patch \
 "
 
 S = "${WORKDIR}/Tremor"
-- 
1.7.11.7

[PATCH 2/2] libatomics-ops: force omit frame pointer for x86 builds

[Jesse Zhang]

Fix failures when building with -fno-omit-frame-pointer (and without
optimization, i.e. -O0):

    In file included from atomic_ops.h:212:0,
                     from atomic_ops_stack.h:32,
                     from atomic_ops_stack.c:23:
    atomic_ops/sysdeps/gcc/x86.h: In function 'AO_compare_double_and_swap_double_full':
    atomic_ops/sysdeps/gcc/x86.h:148:3: error: 'asm' operand has impossible constraints
       __asm__ __volatile__("xchg %%ebx,%6;" /* swap GOT ptr and new_val1 */
       ^

Signed-off-by: Jesse Zhang <sen.zhang@windriver.com>
---
 meta/recipes-multimedia/pulseaudio/libatomics-ops_7.2.bb | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/meta/recipes-multimedia/pulseaudio/libatomics-ops_7.2.bb b/meta/recipes-multimedia/pulseaudio/libatomics-ops_7.2.bb
index f31f983..98e4e0d 100644
--- a/meta/recipes-multimedia/pulseaudio/libatomics-ops_7.2.bb
+++ b/meta/recipes-multimedia/pulseaudio/libatomics-ops_7.2.bb
@@ -20,6 +20,10 @@ ALLOW_EMPTY_${PN} = "1"
 
 ARM_INSTRUCTION_SET = "arm"
 
+# the assembly in src/atomic_ops/sysdeps/gcc/x86.h:AO_compare_double_and_swap_double_full
+# doesn't work with -fno-omit-frame-pointer on x86
+TARGET_CFLAGS_x86 := "${TARGET_CFLAGS} -fomit-frame-pointer"
+
 inherit autotools pkgconfig
 
 do_install_append() {
-- 
1.7.11.7

Re: [PATCH 2/2] libatomics-ops: force omit frame pointer for x86 builds

[Phil Blundell]

On Wed, 2013-07-17 at 16:40 +0800, Jesse Zhang wrote:
> Fix failures when building with -fno-omit-frame-pointer (and without
> optimization, i.e. -O0):
> 
>     In file included from atomic_ops.h:212:0,
>                      from atomic_ops_stack.h:32,
>                      from atomic_ops_stack.c:23:
>     atomic_ops/sysdeps/gcc/x86.h: In function 'AO_compare_double_and_swap_double_full':
>     atomic_ops/sysdeps/gcc/x86.h:148:3: error: 'asm' operand has impossible constraints
>        __asm__ __volatile__("xchg %%ebx,%6;" /* swap GOT ptr and new_val1 */

This seems like it must be a bug in either GCC or the libatomic-ops
source code, and whichever one has the bug ought to be fixed.  Forcing
-fomit-frame-pointer might be ok as a temporary measure but it doesn't
seem like a very good long-term solution.

p.

Re: [PATCH 2/2] libatomics-ops: force omit frame pointer for x86 builds

[Paul Barker]

On 17 July 2013 11:45, Phil Blundell <pb@pbcl.net> wrote:
> On Wed, 2013-07-17 at 16:40 +0800, Jesse Zhang wrote:
>> Fix failures when building with -fno-omit-frame-pointer (and without
>> optimization, i.e. -O0):
>>
>>     In file included from atomic_ops.h:212:0,
>>                      from atomic_ops_stack.h:32,
>>                      from atomic_ops_stack.c:23:
>>     atomic_ops/sysdeps/gcc/x86.h: In function 'AO_compare_double_and_swap_double_full':
>>     atomic_ops/sysdeps/gcc/x86.h:148:3: error: 'asm' operand has impossible constraints
>>        __asm__ __volatile__("xchg %%ebx,%6;" /* swap GOT ptr and new_val1 */
>
> This seems like it must be a bug in either GCC or the libatomic-ops
> source code, and whichever one has the bug ought to be fixed.  Forcing
> -fomit-frame-pointer might be ok as a temporary measure but it doesn't
> seem like a very good long-term solution.
>

If this is on x86, standard Linux desktop/server distros may have ran
into the same problem. Maybe worth having a look if/how they handle
this.

--
Paul Barker

Email: paul@paulbarker.me.uk
http://www.paulbarker.me.uk

Re: [PATCH 2/2] libatomics-ops: force omit frame pointer for x86 builds

[Henning Heinold]

On Wed, Jul 17, 2013 at 12:08:51PM +0100, Paul Barker wrote:
> On 17 July 2013 11:45, Phil Blundell <pb@pbcl.net> wrote:
> > On Wed, 2013-07-17 at 16:40 +0800, Jesse Zhang wrote:
> >> Fix failures when building with -fno-omit-frame-pointer (and without
> >> optimization, i.e. -O0):
> >>
> >>     In file included from atomic_ops.h:212:0,
> >>                      from atomic_ops_stack.h:32,
> >>                      from atomic_ops_stack.c:23:
> >>     atomic_ops/sysdeps/gcc/x86.h: In function 'AO_compare_double_and_swap_double_full':
> >>     atomic_ops/sysdeps/gcc/x86.h:148:3: error: 'asm' operand has impossible constraints
> >>        __asm__ __volatile__("xchg %%ebx,%6;" /* swap GOT ptr and new_val1 */
> >
> > This seems like it must be a bug in either GCC or the libatomic-ops
> > source code, and whichever one has the bug ought to be fixed.  Forcing
> > -fomit-frame-pointer might be ok as a temporary measure but it doesn't
> > seem like a very good long-term solution.
> >
> 
> If this is on x86, standard Linux desktop/server distros may have ran
> into the same problem. Maybe worth having a look if/how they handle
> this.

This makes we wonder which software on x86 relies on libatomic-ops anymore?
gcc buildins should be good enough now.

Bye Henning

Re: [PATCH 2/2] libatomics-ops: force omit frame pointer for x86 builds

[Jesse Zhang]

On 07/17/2013 10:53 PM, Henning Heinold wrote:
> This makes we wonder which software on x86 relies on libatomic-ops anymore?
> gcc buildins should be good enough now.

pulseaudio depends on libatomics-ops though I don't know about the
reasons. The libatomics-ops recipe is actually inside the pulseaudio
dir.

jesse

Re: [PATCH 2/2] libatomics-ops: force omit frame pointer for x86 builds

[Jesse Zhang]

On 07/17/2013 07:08 PM, Paul Barker wrote:
> If this is on x86, standard Linux desktop/server distros may have ran
> into the same problem. Maybe worth having a look if/how they handle
> this.

That's a good idea. I looked but no one seems to have the problem.
Apparently everybody is building with -O2 (which implies
omit-frame-pointer). Gentoo did have a bug report but they used the same
workaround (https://bugs.gentoo.org/show_bug.cgi?id=466860).

But, I looked at the upstream git repo, and it builds fine. I located
two relevant commits and made a patch. Please see the new fix below
(also pushed to my contrib repo).

I'm not sure what the code does exactly, but it at least builds now
with whatever flags.

jesse

From ae44e5c438880d1afada04d06ef0c164c48f8fe9 Mon Sep 17 00:00:00 2001
From: Jesse Zhang <sen.zhang@windriver.com>
Date: Wed, 17 Jul 2013 02:14:02 -0400
Subject: [PATCH] libatomics-ops: backport patch to fix x86 build

Fix failures when building with -fno-omit-frame-pointer (and without
optimization, i.e. -O0):

    In file included from atomic_ops.h:212:0,
                     from atomic_ops_stack.h:32,
                     from atomic_ops_stack.c:23:
    atomic_ops/sysdeps/gcc/x86.h: In function 'AO_compare_double_and_swap_double_full':
    atomic_ops/sysdeps/gcc/x86.h:148:3: error: 'asm' operand has impossible constraints
       __asm__ __volatile__("xchg %%ebx,%6;" /* swap GOT ptr and new_val1 */
       ^

Signed-off-by: Jesse Zhang <sen.zhang@windriver.com>
---
 ...ix-AO_compare_double_and_swap_double_full.patch | 100 +++++++++++++++++++++
 .../pulseaudio/libatomics-ops_7.2.bb               |   1 +
 2 files changed, 101 insertions(+)
 create mode 100644 meta/recipes-multimedia/pulseaudio/libatomics-ops-7.2/fix-AO_compare_double_and_swap_double_full.patch

diff --git a/meta/recipes-multimedia/pulseaudio/libatomics-ops-7.2/fix-AO_compare_double_and_swap_double_full.patch b/meta/recipes-multimedia/pulseaudio/libatomics-ops-7.2/fix-AO_compare_double_and_swap_double_full.patch
new file mode 100644
index 0000000..cb56125
--- /dev/null
+++ b/meta/recipes-multimedia/pulseaudio/libatomics-ops-7.2/fix-AO_compare_double_and_swap_double_full.patch
@@ -0,0 +1,100 @@
+Fix build failure with -fno-omit-frame-pointer
+
+    In file included from atomic_ops.h:212:0,
+                     from atomic_ops_stack.h:32,
+                     from atomic_ops_stack.c:23:
+    atomic_ops/sysdeps/gcc/x86.h: In function 'AO_compare_double_and_swap_double_full':
+    atomic_ops/sysdeps/gcc/x86.h:148:3: error: 'asm' operand has impossible constraints
+       __asm__ __volatile__("xchg %%ebx,%6;" /* swap GOT ptr and new_val1 */
+       ^
+
+Cherry-picked from upstream:
+
+    https://github.com/ivmai/libatomic_ops/commit/613f39d369045e8fc385a439f67a575cddcc6fa1
+    https://github.com/ivmai/libatomic_ops/commit/64d81cd475b07c8a01b91a3be25e20eeca2d27ec
+
+Upstream-Status: backport
+
+Signed-off-by: Jesse Zhang <sen.zhang@windriver.com>
+
+--- a/src/atomic_ops/sysdeps/gcc/x86.h	2013-07-18 02:38:35.182104588 -0400
++++ a/src/atomic_ops/sysdeps/gcc/x86.h	2013-07-18 02:38:46.126104588 -0400
+@@ -137,30 +137,55 @@
+                                        AO_t new_val1, AO_t new_val2)
+ {
+   char result;
+-#if __PIC__
+-  /* If PIC is turned on, we can't use %ebx as it is reserved for the
+-     GOT pointer.  We can save and restore %ebx because GCC won't be
+-     using it for anything else (such as any of the m operands) */
+-  /* We use %edi (for new_val1) instead of a memory operand and swap    */
+-  /* instruction instead of push/pop because some GCC releases have     */
+-  /* a bug in processing memory operands (if address base is %esp) in   */
+-  /* the inline assembly after push.                                    */
+-  __asm__ __volatile__("xchg %%ebx,%6;" /* swap GOT ptr and new_val1 */
+-                       "lock; cmpxchg8b %0; setz %1;"
+-                       "xchg %%ebx,%6;" /* restore ebx and edi */
+-                       : "=m"(*addr), "=a"(result)
+-                       : "m"(*addr), "d" (old_val2), "a" (old_val1),
+-                         "c" (new_val2), "D" (new_val1) : "memory");
+-#else
+-  /* We can't just do the same thing in non-PIC mode, because GCC
+-   * might be using %ebx as the memory operand.  We could have ifdef'd
+-   * in a clobber, but there's no point doing the push/pop if we don't
+-   * have to. */
+-  __asm__ __volatile__("lock; cmpxchg8b %0; setz %1;"
+-                       : "=m"(*addr), "=a"(result)
+-                       : "m"(*addr), "d" (old_val2), "a" (old_val1),
++# ifdef __PIC__
++    AO_t saved_ebx;
++
++    /* If PIC is turned on, we cannot use ebx as it is reserved for the */
++    /* GOT pointer.  We should save and restore ebx.  The proposed      */
++    /* solution is not so efficient as the older alternatives using     */
++    /* push ebx or edi as new_val1 (w/o clobbering edi and temporary    */
++    /* local variable usage) but it is more portable (it works even if  */
++    /* ebx is not used as GOT pointer, and it works for the buggy GCC   */
++    /* releases that incorrectly evaluate memory operands offset in the */
++    /* inline assembly after push).                                     */
++#   ifdef __OPTIMIZE__
++      __asm__ __volatile__("mov %%ebx, %2\n\t" /* save ebx */
++                           "lea %0, %%edi\n\t" /* in case addr is in ebx */
++                           "mov %7, %%ebx\n\t" /* load new_val1 */
++                           "lock; cmpxchg8b (%%edi)\n\t"
++                           "mov %2, %%ebx\n\t" /* restore ebx */
++                           "setz %1"
++                        : "=m" (*addr), "=a" (result), "=m" (saved_ebx)
++                        : "m" (*addr), "d" (old_val2), "a" (old_val1),
++                          "c" (new_val2), "m" (new_val1)
++                        : "%edi", "memory");
++#   else
++      /* A less-efficient code manually preserving edi if GCC invoked   */
++      /* with -O0 option (otherwise it fails while finding a register   */
++      /* in class 'GENERAL_REGS').                                      */
++      AO_t saved_edi;
++      __asm__ __volatile__("mov %%edi, %3\n\t" /* save edi */
++                           "mov %%ebx, %2\n\t" /* save ebx */
++                           "lea %0, %%edi\n\t" /* in case addr is in ebx */
++                           "mov %8, %%ebx\n\t" /* load new_val1 */
++                           "lock; cmpxchg8b (%%edi)\n\t"
++                           "mov %2, %%ebx\n\t" /* restore ebx */
++                           "mov %3, %%edi\n\t" /* restore edi */
++                           "setz %1"
++                        : "=m" (*addr), "=a" (result),
++                          "=m" (saved_ebx), "=m" (saved_edi)
++                        : "m" (*addr), "d" (old_val2), "a" (old_val1),
++                          "c" (new_val2), "m" (new_val1) : "memory");
++#   endif
++# else
++    /* For non-PIC mode, this operation could be simplified (and be     */
++    /* faster) by using ebx as new_val1 (GCC would refuse to compile    */
++    /* such code for PIC mode).                                         */
++    __asm__ __volatile__("lock; cmpxchg8b %0; setz %1"
++                       : "=m" (*addr), "=a" (result)
++                       : "m" (*addr), "d" (old_val2), "a" (old_val1),
+                          "c" (new_val2), "b" (new_val1) : "memory");
+-#endif
++# endif
+   return (int) result;
+ }
+ #define AO_HAVE_compare_double_and_swap_double_full
diff --git a/meta/recipes-multimedia/pulseaudio/libatomics-ops_7.2.bb b/meta/recipes-multimedia/pulseaudio/libatomics-ops_7.2.bb
index f31f983..53e6154 100644
--- a/meta/recipes-multimedia/pulseaudio/libatomics-ops_7.2.bb
+++ b/meta/recipes-multimedia/pulseaudio/libatomics-ops_7.2.bb
@@ -9,6 +9,7 @@ LIC_FILES_CHKSUM = "file://doc/COPYING;md5=94d55d512a9ba36caa9b7df079bae19f \
 PR = "r1"
 
 SRC_URI = "http://www.hpl.hp.com/research/linux/atomic_ops/download/libatomic_ops-${PV}.tar.gz \
+           file://fix-AO_compare_double_and_swap_double_full.patch \
           "
 
 SRC_URI[md5sum] = "890acdc83a7cd10e2e9536062d3741c8"
-- 
1.7.11.7

Re: [PATCH 2/2] libatomics-ops: force omit frame pointer for x86 builds

[Paul Barker]

On 18 July 2013 09:57, Jesse Zhang <sen.zhang@windriver.com> wrote:
>
> But, I looked at the upstream git repo, and it builds fine. I located
> two relevant commits and made a patch. Please see the new fix below
> (also pushed to my contrib repo).
>
> I'm not sure what the code does exactly, but it at least builds now
> with whatever flags.

If you want to know, see
http://en.wikipedia.org/wiki/Position-independent_code for the meaning
of PIC and GOT in this context. The rest is probably just gcc
weirdness and the way it uses registers to store the frame pointer.

--
Paul Barker

Email: paul@paulbarker.me.uk
http://www.paulbarker.me.uk

Re: [PATCH 2/2] libatomics-ops: force omit frame pointer for x86 builds

[Henning Heinold]

On Thu, Jul 18, 2013 at 04:46:13PM +0800, Jesse Zhang wrote:
> On 07/17/2013 10:53 PM, Henning Heinold wrote:
> > This makes we wonder which software on x86 relies on libatomic-ops anymore?
> > gcc buildins should be good enough now.
> 
> pulseaudio depends on libatomics-ops though I don't know about the
> reasons. The libatomics-ops recipe is actually inside the pulseaudio
> dir.
> 
> jesse

Hm yes, I still feared it.

pulesaudio needed libatomic-ops only for mips architecture. I looked that up a
long time ago. Now with 2.0 release of pulseaudio and newer releases of gcc I believe
libatomic-ops is not needed anymore for pulseaudio.

Bye Henning