This is the mail archive of the
mailing list for the GCC project.
Re: [committed] Update config/pa/linux-atomic.c and enable sync tests on hppa-linux
- From: Richard Henderson <rth at redhat dot com>
- To: John David Anglin <dave dot anglin at nrc-cnrc dot gc dot ca>
- Cc: John David Anglin <dave at hiauly1 dot hia dot nrc dot ca>, gcc-patches at gcc dot gnu dot org
- Date: Wed, 11 Aug 2010 11:03:11 -0700
- Subject: Re: [committed] Update config/pa/linux-atomic.c and enable sync tests on hppa-linux
- References: <20100811022808.GA25554@hiauly1.hia.nrc.ca>
On 08/10/2010 07:28 PM, John David Anglin wrote:
> The attached patches are essentially identical to that recently applied
> to arm.
> Tested on hppa-unknown-linux-gnu with no observed regressions. Committed
> to trunk. Plan to backport to 4.4 and 4.5 after testing.
FWIW, I had a browse through this file and it's a bit off. E.g.:
> __sync_val_compare_and_swap_4 (int *ptr, int oldval, int newval)
> int actual_oldval, fail;
> while (1)
> actual_oldval = *ptr;
> if (oldval != actual_oldval)
> return actual_oldval;
> fail = __kernel_cmpxchg (actual_oldval, newval, ptr);
> if (!fail)
> return oldval;
This should not be returning the OLDVAL that the user passed,
it should be returning the lws_ret that the kernel returned.
This is because the user of __sync_val_c_a_s wants avoid the
reload of the value, like so (from libgomp/iter.c):
> tmp = __sync_val_compare_and_swap (&ws->next, start, nend);
> if (__builtin_expect (tmp == start, 1))
> start = tmp;
You might be better writing
__kernel_cmpxchg (int oldval, int newval, int *mem)
} while (lws_errno == -EAGAIN);
/* Other possibilities are EFAULT, ENOSYS, EDEADLOCK. */
if (__builtin_expect (lws_errno != 0, 0))
and totally hiding the possibility of kernel failure.
This, incidentally, is exactly the implementation of
__sync_val_c_a_s, with arguments in a different order.