This is the mail archive of the mailing list for the GCC project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [committed] Update config/pa/linux-atomic.c and enable sync tests on hppa-linux

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))
>         break;
>       start = tmp;

You might be better writing

static int
__kernel_cmpxchg (int oldval, int newval, int *mem)
  do {
  } while (lws_errno == -EAGAIN);

  /* Other possibilities are EFAULT, ENOSYS, EDEADLOCK.  */
  if (__builtin_expect (lws_errno != 0, 0))
      __builtin_unreachable ();
  return lws_ret;

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.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]