[Bug target/105011] New: [nvptx] FAIL: gcc.dg/atomic/stdatomic-flag-2.c -O1 execution test

[Bug target/105011] New: [nvptx] FAIL: gcc.dg/atomic/stdatomic-flag-2.c -O1 execution test

[vries at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105011

            Bug ID: 105011
           Summary: [nvptx] FAIL: gcc.dg/atomic/stdatomic-flag-2.c   -O1
                    execution test
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: vries at gcc dot gnu.org
  Target Milestone: ---

On a quadro k2000 with driver 470.103.01 I'm running into this cluster of
FAILs:
...
FAIL: gcc.dg/atomic/stdatomic-flag-2.c   -O1  execution test
FAIL: gcc.dg/atomic/stdatomic-flag-2.c   -O3 -g  execution test
FAIL: gcc.dg/atomic/stdatomic-flag.c   -O3 -g  execution test
...

Minimizing the first FAIL, I end up with:
...
#include <stdatomic.h>

extern void abort (void);
atomic_flag a = ATOMIC_FLAG_INIT;

int
main ()
{
  int b;

  if ((atomic_flag_test_and_set) (&a))
    abort ();

  return 0;
}
...

The atomic access is done by libatomic, using a 64-bit cas loop.

If we print the address of a, we have:
...
&a: 0000000700700200
...
so the pointer is already 64-bit aligned.

If we print the 64-bit value of *(unsigned long long *)&a before and after the
test-and-set, we have:
...
a: 0000000000024f00
a: 0000000000024f01
...
so that looks all-right as well.

At first glance, the problem is in libatomic, tas_n.c:
...
  wval = (UWORD)__GCC_ATOMIC_TEST_AND_SET_TRUEVAL << shift;
  woldval = __atomic_load_n (wptr, __ATOMIC_RELAXED);
  do
    {
      t = woldval | wval;
    }
  while (!atomic_compare_exchange_w (wptr, &woldval, t, true,
                                     __ATOMIC_RELAXED, __ATOMIC_RELAXED));

  post_barrier (smodel);
  return woldval != 0;
...

What is returned is woldval != 0, but that tests the entire word, not just the
byte we're interested in.

It should probably do something like:
...
  return (woldval & wval) != 0;
...

[Bug target/105011] [nvptx] FAIL: gcc.dg/atomic/stdatomic-flag-2.c -O1 execution test

[vries at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105011

--- Comment #1 from Tom de Vries <vries at gcc dot gnu.org> ---
(In reply to Tom de Vries from comment #0)
> It should probably do something like:
> ...
>   return (woldval & wval) != 0;
> ...

Indeed, that fixes the FAILs.

[Bug target/105011] [nvptx] FAIL: gcc.dg/atomic/stdatomic-flag-2.c -O1 execution test

[vries at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105011

--- Comment #2 from Tom de Vries <vries at gcc dot gnu.org> ---
Even better:
...
diff --git a/libatomic/tas_n.c b/libatomic/tas_n.c
index d0d8c283b495..65eaa7753a51 100644
--- a/libatomic/tas_n.c
+++ b/libatomic/tas_n.c
@@ -73,7 +73,7 @@ SIZE(libat_test_and_set) (UTYPE *mptr, int smodel)
                                     __ATOMIC_RELAXED, __ATOMIC_RELAXED));

   post_barrier (smodel);
-  return woldval != 0;
+  return (woldval & wval) == wval;
 }

 #define DONE 1
...

That also gives back accurate results in case
TARGET_ATOMIC_TEST_AND_SET_TRUEVAL has more than one bit set.

[Bug target/105011] [nvptx] FAIL: gcc.dg/atomic/stdatomic-flag-2.c -O1 execution test

[vries at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105011

--- Comment #3 from Tom de Vries <vries at gcc dot gnu.org> ---
Submitted fix :
https://gcc.gnu.org/pipermail/gcc-patches/2022-March/592211.html

Though without changelog, apparently.

[Bug target/105011] [nvptx] FAIL: gcc.dg/atomic/stdatomic-flag-2.c -O1 execution test

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105011

--- Comment #4 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Tom de Vries <vries@gcc.gnu.org>:

https://gcc.gnu.org/g:11fb784ac592567dbcb7874c27e67ee0feb8fbf0

commit r12-7799-g11fb784ac592567dbcb7874c27e67ee0feb8fbf0
Author: Tom de Vries <tdevries@suse.de>
Date:   Wed Mar 23 16:37:45 2022 +0100

    [libatomic] Fix return value in libat_test_and_set

    On nvptx (using a Quadro K2000 with driver 470.103.01) I ran into this:
    ...
    FAIL: gcc.dg/atomic/stdatomic-flag-2.c -O1 execution test
    ...
    which mimimized to:
    ...
      #include <stdatomic.h>
      atomic_flag a = ATOMIC_FLAG_INIT;
      int main () {
        if ((atomic_flag_test_and_set) (&a))
          __builtin_abort ();
        return 0;
      }
    ...

    The atomic_flag_test_and_set is implemented using __atomic_test_and_set_1,
    which corresponds to the "word-sized compare-and-swap loop" version of
    libat_test_and_set in libatomic/tas_n.c.

    The semantics of a test-and-set is that the return value is "true if and
only
    if the previous contents were 'set'".

    But the code uses:
    ...
      return woldval != 0;
    ...
    which means it doesn't look only at the byte that was either set or not
set,
    but at the entire word.

    Fix this by using instead:
    ...
      return (woldval & ((UTYPE) ~(UTYPE) 0 << shift)) != 0;
    ...

    Tested on nvptx.

    libatomic/ChangeLog:

    2022-03-24  Tom de Vries  <tdevries@suse.de>

            PR target/105011
            * tas_n.c (libat_test_and_set): Fix return value.

[Bug target/105011] [nvptx] FAIL: gcc.dg/atomic/stdatomic-flag-2.c -O1 execution test

[vries at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105011

Tom de Vries <vries at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
   Target Milestone|---                         |12.0
         Resolution|---                         |FIXED

--- Comment #5 from Tom de Vries <vries at gcc dot gnu.org> ---
Fixed by commit.