Re: test_stack on powerpc (power7)

On Thu, Jan 30, 2014 at 12:56:03PM -0600, Will Schmidt wrote:
> On Wed, 2014-01-29 at 07:05 +0000, Boehm, Hans wrote:
> > Interesting.
> <...snip...>
> 
> > I would also check that the recheck of first against AO_load(list) appears in the assembly code where it should.
> 
> Thanks for the suggestions. :-)
> 
> I looked closer at the code that does the x_bits twiddling, and after a
> bit of quality time single-stepping within gdb, have found an issue.
> The AO_load() function is being mapped to AO_load() in
> sysdeps/loadstore/atomic_load.h, rather than the AO_load_acquire() in
> powerpc.h like I had initialy thought, per the #defines I was lookging
> at in generalize-small.h.  The critical detail in that is the lack of an
> isync in the atomic_load.h version.
> 
> I'm admittedly unclear of how the path through the header file includes
> should be.
> 
> The patch below (inline and attached) seems a bit hackish to me, but is
> also sufficient to allow test_stack to run to completion on the P7 here.
> (test_stack running in a loop, ~ 1000 successful runs so far).   For
> inclusion as-is, or as inspiration to whomever better understands the
> include hierarchy.
> 
> Thanks, 
> -Will
> 
> --
> 
> Force AO_load() to map to AO_load_acquire() for powerpc.  The
> AO_load_acquire() function includes isync instructions that
> are critical for proper behavior on power system.
> 
> 
> Signed-Off-By: Will Schmidt <will_schmidt-DhXWPJtHtuFWk0Htik3J/[email protected]>
> 
> 
> diff -aur --exclude='*.Plo' --exclude='*.Po' libatomic_ops-7.4.0/src/atomic_ops/sysdeps/gcc/powerpc.h libatomic_ops-7.4.0.new/src/atomic_ops/sysdeps/gcc/powerpc.h
> --- libatomic_ops-7.4.0/src/atomic_ops/sysdeps/gcc/powerpc.h	2013-11-10 03:57:17.000000000 -0600
> +++ libatomic_ops-7.4.0.new/src/atomic_ops/sysdeps/gcc/powerpc.h	2014-01-30 12:17:20.819984940 -0600
> @@ -29,6 +29,8 @@
>  
>  #include "../all_aligned_atomic_load_store.h"
>  
> +#define AO_load(addr) AO_load_acquire(addr)
> +
>  #include "../test_and_set_t_is_ao_t.h"
>          /* There seems to be no byte equivalent of lwarx, so this       */
>          /* may really be what we want, at least in the 32-bit case.     */
> 
> 
> 

> diff -aur --exclude='*.Plo' --exclude='*.Po' libatomic_ops-7.4.0/src/atomic_ops/sysdeps/gcc/powerpc.h libatomic_ops-7.4.0.new/src/atomic_ops/sysdeps/gcc/powerpc.h
> --- libatomic_ops-7.4.0/src/atomic_ops/sysdeps/gcc/powerpc.h	2013-11-10 03:57:17.000000000 -0600
> +++ libatomic_ops-7.4.0.new/src/atomic_ops/sysdeps/gcc/powerpc.h	2014-01-30 12:17:20.819984940 -0600
> @@ -29,6 +29,8 @@
>  
>  #include "../all_aligned_atomic_load_store.h"
>  
> +#define AO_load(addr) AO_load_acquire(addr)
> +
>  #include "../test_and_set_t_is_ao_t.h"
>          /* There seems to be no byte equivalent of lwarx, so this       */
>          /* may really be what we want, at least in the 32-bit case.     */

Works for me on the version in Debian Wheezy
(libatomic-ops-7.2~alpha5+cvs20100601).

Yayyyy!

-- 
Len Sorensen