[Gc] Re[12]: PATCH: Add x32 support to boehm-gc

Ivan Maidanski ivmai at mail.ru
Mon Oct 8 09:05:00 PDT 2012


Hi H.J.,

Mon, 8 Oct 2012 08:54:08 -0700 от "H.J. Lu" <hjl.tools at gmail.com>:
>A couple comments:
>
>
1. AO_double_compare_and_swap_full is wrong:
>
>
#elif defined(__ILP32__) || !defined(__x86_64__)
>
# include "../standard_ao_double_t.h"
>
>
  /* X32 has native support for 64-bit integer operations (AO_double_t  */
>
  /* is a 64-bit integer and we could use 64-bit cmpxchg).              */
>
  /* This primitive is used by compare_double_and_swap_double_full.     */
>
  AO_INLINE int
>
  AO_double_compare_and_swap_full(volatile AO_double_t *addr,
>
                                  AO_double_t old_val, AO_double_t new_val)
>
>
This is only for x32.  You should check
>
>
#elif defined(__ILP32__) && defined(__x86_64__)
No, the former is correct. Because this is not only for x32 but also for x86 (if gcc 4+ and AO_USE_SYNC_CAS_BUILTIN manually defined). I've tested it (and inspected -S output) with recent clang and recent gcc-4.4 .. 4.7. See this commit message: https://github.com/ivmai/libatomic_ops/commit/03de7740c21fe6e4a6bdd7af09d5ff5189d4d70e


>
>
2. Why is AO_int_fetch_and_add_full only defined for 64-bit x86-64?
>
>
  AO_INLINE unsigned int
>
  AO_int_fetch_and_add_full (volatile unsigned int *p, unsigned int incr)
>
  {
>
    unsigned int result;
>
>
    __asm__ __volatile__ ("lock; xaddl %0, %1"
>
                        : "=r" (result), "=m" (*p)
>
                        : "0" (incr), "m" (*p)
>
                        : "memory");
>
    return result;
>
  }
>
>
works for ia32, x32 and x86-64.For ia32 and x32, we define AO_T_IS_INT which force AO_int_fetch_and_add_full to be defined as AO_fetch_and_add_full in ao_t_is_int.h

Please run test_atomic and see it output whether AO_int_fetch_and_add_full is reported to be missing on X32. I'm sure it's not.

Does "make check" successfully pass all tests on X32?

Regards,
Ivan

>
>
On Mon, Oct 8, 2012 at 8:28 AM, Ivan Maidanski <ivmai at mail.ru> wrote:
>
> Hi H.J.,
>
>
>
> Fix AO_compare_double_and_swap_double_full for x32 in this branch:
>
> https://github.com/ivmai/libatomic_ops/tree/fix-double-cas-x32 (diff against
>
> master:
>
> https://github.com/ivmai/libatomic_ops/compare/master...fix-double-cas-x32)
>
> Please test it.
>
>
>
> In fact, gcc/x86.h defines only double_compare_and_swap for x32 and
>
> compare_double_and_swap_double is defined now in generalize.h.
>
>
>
> (Sorry for the delay but I've applied a dozen of patches related to code
>
> refactoring, double_cas testing (and improvement of MS VC code) these days -
>
> https://github.com/ivmai/libatomic_ops/compare/c6dc43294bda5516dea392cd4d994cbd1a18288e...dcf66b7de2f053d4c27fe96e6c7587f1fb183209)
>
>
>
>
>
>
>
> Regards,
>
> Ivan
>
>
>
> Mon, 1 Oct 2012 06:30:57 -0700 "H.J. Lu" <hjl.tools at gmail.com>:
>
>
>
> On Mon, Oct 1, 2012 at 5:54 AM, Ivan Maidanski <ivmai at mail.ru> wrote:
>
>> Hi H.J,
>
>>
>
>> Done both (in the same branch). Please retry.
>
>>
>
>> I've implemented double CAS using __sync_bool_compare_and_swap (similar to
>
>> normal CAS on x86_64 starting from GCC 4.2). Please inspect the assembly
>
>> code for it (as you pointed out it should be cmpxchg but not cmpxchg8b).
>
>> Also, I don't add cast of __sync_bool_compare_and_swap result to int (I
>
>> assume the problem should be already fixed on GCC with x32 support -
>
>> please
>
>> check that GCC does not report warnings in
>
>> AO_double_compare_and_swap_full).
>
>
>
> AO_compare_double_and_swap_double_full is wrong for x32. Why do we
>
> need it for x32 since x32 has AO_double_compare_and_swap_full?
>
>
>
> --
>
> H.J.
>
>
>
>
-- 
>
H.J.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://napali.hpl.hp.com/pipermail/gc/attachments/20121008/18a7cb63/attachment.htm


More information about the Gc mailing list