[Bug c/37908] New: atomic NAND op generate wrong code; __sync_nand_and_fetch, __sync_fetch_and_nand

[Bug c/37908] New: atomic NAND op generate wrong code; __sync_nand_and_fetch, __sync_fetch_and_nand

[kokseng at ieee dot org]

Hello,

I discovered a failure relating to atomic NAND operation on  x86 platform,
which can be confirmed using the included test-case.  The test was done with
gcc 4.2.4, using 8-, 16-, 32- and 64-bit data width.  Test cases on other logic
atomic operators passes all width-size except NAND.  I have read through the
assembly code generated and confirm that some wrong codes are generated when
no-optimization and -O.


I ) Test case : 

/*
        Compile using:
                gcc -c -march=i686 test.c 

        To get assembly listing:
                gcc -S -march=i686 test.c

        To get mix c and assembly listing:
                gcc -c -g -Wa,-a,-ad -march=i686 test.c > test-xxx.lst
*/

#include <stddef.h>
#include <stdlib.h>
#include <stdio.h>

typedef unsigned char BYTE;
typedef unsigned short WORD;
typedef unsigned long DWORD;
typedef unsigned long long QWORD;


int main(void)
{

        WORD xLoc;  /* change xLoc type to BYTE, WORD, DWORD, QWORD to test
different data-width */
        typeof(xLoc)    xIn, xOut, xExpect, i = 1; 
        xLoc = xIn = (typeof(xIn)) ~ (1<<i); 
        xExpect = (typeof(xIn)) ~(xIn & ((typeof(xIn)) 0x7F)); 
        /* Both __sync_nand_and_fetch and __sync_fetch_and_nand have the same
problem */
        xOut = __sync_nand_and_fetch(&xLoc, (typeof(xIn)) 0x7F);
        if (xOut!=xExpect) 
                printf("__sync_nand_and_fetch():; wrong result; i(%d) xIn(%x)
xExpect(%x) xOut(%x) xLoc(%x)\n", 
                        i, xIn, xExpect, xOut, xLoc);

        return 0;
}

II )  Output of gcc -v -save-temps

gcc -o test_bug -v -save-temps -march=i686 test_nand_bug.c

Using built-in specs.
Target: i686-pc-linux-gnu
Configured with: ../gcc-4.2.4/configure --prefix=/opt/gcc-4.2.4
--host=i686-pc-linux-gnu --target=i686-pc-linux-gnu
Thread model: posix
gcc version 4.2.4
 /opt/gcc-4.2.4/libexec/gcc/i686-pc-linux-gnu/4.2.4/cc1 -E -quiet -v
test_nand_bug.c -march=i686 -fpch-preprocess -o test_nand_bug.i
ignoring nonexistent directory
"/opt/gcc-4.2.4/lib/gcc/i686-pc-linux-gnu/4.2.4/../../../../i686-pc-linux-gnu/include"
#include "..." search starts here:
#include <...> search starts here:
 /usr/local/include
 /opt/gcc-4.2.4/include
 /opt/gcc-4.2.4/lib/gcc/i686-pc-linux-gnu/4.2.4/include
 /usr/include
End of search list.
 /opt/gcc-4.2.4/libexec/gcc/i686-pc-linux-gnu/4.2.4/cc1 -fpreprocessed
test_nand_bug.i -quiet -dumpbase test_nand_bug.c -march=i686 -auxbase
test_nand_bug -version -o test_nand_bug.s
GNU C version 4.2.4 (i686-pc-linux-gnu)
        compiled by GNU C version 4.2.4.
GGC heuristics: --param ggc-min-expand=47 --param ggc-min-heapsize=32052
Compiler executable checksum: 383821529167afc2e47a93836e3831a4
 as -V -Qy -o test_nand_bug.o test_nand_bug.s
GNU assembler version 2.13.90.0.18 (i386-redhat-linux) using BFD version
2.13.90.0.18 20030206
 /opt/gcc-4.2.4/libexec/gcc/i686-pc-linux-gnu/4.2.4/collect2 --eh-frame-hdr -m
elf_i386 -dynamic-linker /lib/ld-linux.so.2 -o test_bug /usr/lib/crt1.o
/usr/lib/crti.o /opt/gcc-4.2.4/lib/gcc/i686-pc-linux-gnu/4.2.4/crtbegin.o
-L/opt/gcc-4.2.4/lib/gcc/i686-pc-linux-gnu/4.2.4
-L/opt/gcc-4.2.4/lib/gcc/i686-pc-linux-gnu/4.2.4/../../.. test_nand_bug.o -lgcc
-lgcc_eh -lc -lgcc -lgcc_eh
/opt/gcc-4.2.4/lib/gcc/i686-pc-linux-gnu/4.2.4/crtend.o /usr/lib/crtn.o


-- 
           Summary: atomic NAND op generate wrong code;
                    __sync_nand_and_fetch, __sync_fetch_and_nand
           Product: gcc
           Version: 4.2.4
            Status: UNCONFIRMED
          Severity: major
          Priority: P3
         Component: c
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: kokseng at ieee dot org


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=37908

[Bug middle-end/37908] [4.2/4.3/4.4 regression] atomic NAND op generate wrong code; __sync_nand_and_fetch, __sync_fetch_and_nand

[ubizjak at gmail dot com]

------- Comment #1 from ubizjak at gmail dot com  2008-10-24 07:44 -------
Confirmed.


-- 

ubizjak at gmail dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|c                           |middle-end
            Summary|atomic NAND op generate     |[4.2/4.3/4.4 regression]
                   |wrong code;                 |atomic NAND op generate
                   |__sync_nand_and_fetch,      |wrong code;
                   |__sync_fetch_and_nand       |__sync_nand_and_fetch,
                   |                            |__sync_fetch_and_nand
   Target Milestone|---                         |4.2.5


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=37908

[Bug middle-end/37908] [4.2/4.3/4.4 regression] atomic NAND op generate wrong code; __sync_nand_and_fetch, __sync_fetch_and_nand

[ubizjak at gmail dot com]

------- Comment #2 from ubizjak at gmail dot com  2008-10-24 07:44 -------
Patch in testing.


-- 

ubizjak at gmail dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|unassigned at gcc dot gnu   |ubizjak at gmail dot com
                   |dot org                     |
             Status|UNCONFIRMED                 |ASSIGNED
     Ever Confirmed|0                           |1
   Last reconfirmed|0000-00-00 00:00:00         |2008-10-24 07:44:56
               date|                            |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=37908

[Bug middle-end/37908] [4.2/4.3/4.4 regression] atomic NAND op generate wrong code; __sync_nand_and_fetch, __sync_fetch_and_nand

[jakub at gcc dot gnu dot org]

------- Comment #3 from jakub at gcc dot gnu dot org  2008-10-24 10:42 -------
__sync_*nand* behaves as documented.  See info gcc, Atomic Builtins.
or
http://gcc.gnu.org/ml/gcc-patches/2005-04/msg02168.html
Do you have any evidence that icc implements this differently?


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=37908

[Bug middle-end/37908] [4.2/4.3/4.4 regression] atomic NAND op generate wrong code; __sync_nand_and_fetch, __sync_fetch_and_nand

[kokseng at ieee dot org]

------- Comment #4 from kokseng at ieee dot org  2008-10-24 12:18 -------
Okay, now I noticed the 'nand' comment on the documentation for atomic
builtins, the code does implement the 'negate and AND' logic, which is named
'nand'.  

On page 164 of icc  "Intrinsics Reference"
(http://softwarecommunity.intel.com/isn/downloads/softwareproducts/pdfs/347603.pdf),
ti does not have such qualifier that 'nand' is not NAND, but negate-and-AND. 

I do not have any further evidence how icc actually implement __sync_*nand*. 

This came about when adding unit-test on atomic operation for purpose of
trapping problems when porting across platform.  

I wonder why the overloading of 'nand' ?



-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=37908

[Bug middle-end/37908] [4.2/4.3/4.4 regression] atomic NAND op generate wrong code; __sync_nand_and_fetch, __sync_fetch_and_nand

[ubizjak at gmail dot com]

------- Comment #5 from ubizjak at gmail dot com  2008-10-24 16:47 -------
(In reply to comment #4)
> Okay, now I noticed the 'nand' comment on the documentation for atomic
> builtins, the code does implement the 'negate and AND' logic, which is named
> 'nand'.  

So, INVALID.


-- 

ubizjak at gmail dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |INVALID


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=37908

[Bug middle-end/37908] [4.2/4.3/4.4 regression] atomic NAND op generate wrong code; __sync_nand_and_fetch, __sync_fetch_and_nand

[ubizjak at gmail dot com]

------- Comment #6 from ubizjak at gmail dot com  2008-10-29 07:10 -------
According to Intel [1]:

According to Dan, __sync_fetch_and_nand intrinsic should be implemented as
~(target & val). Uros's patch is correct.

[1] http://gcc.gnu.org/ml/gcc-patches/2008-10/msg01214.html


-- 

ubizjak at gmail dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|major                       |critical
             Status|RESOLVED                    |REOPENED
           Keywords|                            |wrong-code
           Priority|P3                          |P1
         Resolution|INVALID                     |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=37908

[Bug middle-end/37908] [4.2/4.3/4.4 regression] atomic NAND op generate wrong code; __sync_nand_and_fetch, __sync_fetch_and_nand

[kokseng at ieee dot org]

------- Comment #7 from kokseng at ieee dot org  2008-10-29 09:37 -------
The only problem is whether there are codes out there that depend on
"NEGATE-and-AND"?     Frankly speaking,  when I was reminded about the 'nand'
comment on gcc manual, only then I remembered many moons ago, I read that line,
and also remembered saying to myself "W-T-F".  However, many moons later, when
I work on porting to other processor and test-the-unit-test, I relied on
text-book and historical definition of the term 'NAND'.  So, I think it is not
an issue of what Intel ICC did, it is matter of getting the semantic of NAND
the way any programmer was taught and practiced.

Propagating a misnomer will only guarantee that this issue of " NAND is not
NAND but 'N'AND" will come back to haunt every now-and-then.  IMHO, it is
better to bite the bullet now then swallow a canon later.

 :-)


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=37908

[Bug middle-end/37908] [4.2/4.3/4.4 regression] atomic NAND op generate wrong code; __sync_nand_and_fetch, __sync_fetch_and_nand

[ubizjak at gmail dot com]

------- Comment #8 from ubizjak at gmail dot com  2008-10-29 10:55 -------
Patch at http://gcc.gnu.org/ml/gcc-patches/2008-10/msg01224.html


-- 

ubizjak at gmail dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                URL|                            |http://gcc.gnu.org/ml/gcc-
                   |                            |patches/2008-
                   |                            |10/msg01224.html
             Status|REOPENED                    |ASSIGNED
   Last reconfirmed|2008-10-24 07:44:56         |2008-10-29 10:55:33
               date|                            |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=37908

[Bug middle-end/37908] atomic NAND op generate wrong code; __sync_nand_and_fetch, __sync_fetch_and_nand

[jakub at gcc dot gnu dot org]

------- Comment #9 from jakub at gcc dot gnu dot org  2008-11-03 11:24 -------
Removing regression flag, as this isn't a regression, the intrinsics worked
that way forever.  That doesn't mean it shouldn't be changed for 4.4, for 4.3
and earlier IMHO only a warning is acceptable, not changing semantics of it.


-- 

jakub at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[4.2/4.3/4.4 regression]    |atomic NAND op generate
                   |atomic NAND op generate     |wrong code;
                   |wrong code;                 |__sync_nand_and_fetch,
                   |__sync_nand_and_fetch,      |__sync_fetch_and_nand
                   |__sync_fetch_and_nand       |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=37908

[Bug middle-end/37908] atomic NAND op generate wrong code; __sync_nand_and_fetch, __sync_fetch_and_nand

[ubizjak at gmail dot com]

------- Comment #10 from ubizjak at gmail dot com  2008-11-05 13:22 -------
Next revision of the patch (v3) at [1] generates a message when nand builtin is
encountered.

[1] http://gcc.gnu.org/ml/gcc-patches/2008-11/msg00157.html 


-- 

ubizjak at gmail dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                URL|http://gcc.gnu.org/ml/gcc-  |http://gcc.gnu.org/ml/gcc-
                   |patches/2008-               |patches/2008-
                   |10/msg01224.html            |11/msg00157.html


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=37908

[Bug middle-end/37908] atomic NAND op generate wrong code; __sync_nand_and_fetch, __sync_fetch_and_nand

[uros at gcc dot gnu dot org]

------- Comment #11 from uros at gcc dot gnu dot org  2008-11-17 11:20 -------
Subject: Bug 37908

Author: uros
Date: Mon Nov 17 11:19:06 2008
New Revision: 141942

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=141942
Log:
        PR middle-end/37908
        * optabs.c (expand_sync_operation): Properly handle NAND case
        by calculating ~(t1 & val) instead of (~t1 & val).
        * builtins.c (expand_builtin_sync_operation): Warn for changed
        semantics in NAND builtins.
        * c.opt (Wsync-nand): New warning option.  Describe -Wsync-nand.

        * doc/invoke.texi (Warning options): Add Wsync-nand.
        * doc/extend.texi (Atomic Builtins) [__sync_fetch_and_nand]: Correct
        __sync_fetch_and_nand builtin operation in the example.  Add a note
        about changed semantics in GCC 4.4.
        [__sync_nand_and_fetch]: Correct __sync_nand_and_fetch builtin
        operation in the example.  Add a note about changed semantics in
        GCC 4.4.

testsuite/ChangeLog:

        PR middle-end/37908
        * gcc.dg/pr37908.c: New test.
        * gcc.dg/ia64-sync-1.c: Correct __sync_fetch_and_nand and
        __sync_nand_and_fetch results.  Add dg-message to look for the warning
        about changed semantics of NAND builtin.
        (init_si, init_di): Change init value for __sync_fetch_and_nand to -1.
        (test_si, test_di): Change expected result of
        __sync_nand_and_fetch to ~7.
        * gcc.dg/ia64-sync-2.c: Correct __sync_fetch_and_nand and
        __sync_nand_and_fetch results.  Add dg-message to look for the warning
        about changed semantics of NAND builtin.
        (init_noret_si, init_noret_di): Change init value for
        __sync_fetch_and_nand to -1.
        (init_noret_si, init_noret_di): Change expected result of
        __sync_nand_and_fetch to ~7.
        * gcc.dg/sync-2.c: Correct __sync_fetch_and_nand and
        __sync_nand_and_fetch results.  Add dg-message to look for the warning
        about changed semantics of NAND builtin.
        (init_qi, init_qi): Change init value for __sync_fetch_and_nand to -1.
        (init_hi, init_hi): Change expected result of
        __sync_nand_and_fetch to ~7.
        * gcc.dg/sync-3.c: Copy from sync-2.c instead of including
        the c source file.
        * gcc.c-torture/compile/sync-1.c: Add dg-message to look for the
        warning about changed semantics of NAND builtin.
        * gcc.c-torture/compile/sync-2.c: Ditto.
        * gcc.c-torture/compile/sync-3.c: Ditto.


Added:
    trunk/gcc/testsuite/gcc.dg/pr37908.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/builtins.c
    trunk/gcc/c.opt
    trunk/gcc/doc/extend.texi
    trunk/gcc/doc/invoke.texi
    trunk/gcc/optabs.c
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/gcc.c-torture/compile/sync-1.c
    trunk/gcc/testsuite/gcc.c-torture/compile/sync-2.c
    trunk/gcc/testsuite/gcc.c-torture/compile/sync-3.c
    trunk/gcc/testsuite/gcc.dg/ia64-sync-1.c
    trunk/gcc/testsuite/gcc.dg/ia64-sync-2.c
    trunk/gcc/testsuite/gcc.dg/sync-2.c
    trunk/gcc/testsuite/gcc.dg/sync-3.c


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=37908

[Bug middle-end/37908] atomic NAND op generate wrong code; __sync_nand_and_fetch, __sync_fetch_and_nand

[uros at gcc dot gnu dot org]

------- Comment #12 from uros at gcc dot gnu dot org  2008-11-21 07:29 -------
Subject: Bug 37908

Author: uros
Date: Fri Nov 21 07:28:27 2008
New Revision: 142082

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=142082
Log:
        PR middle-end/37908
        * config/ia64/ia64.c (ia64_expand_atomic_ope): Properly handle NAND
        case by calculating ~(new_reg & val) instead of (~new_reg & val).
        * config/ia64/sync.md (sync_nand<mode>): Change insn RTX
        to (not:IMODE (and:IMODE (...))).
        (sync_old_nand<mode>): Ditto.
        (sync_new_nand<mode>): Ditto.


Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/ia64/ia64.c
    trunk/gcc/config/ia64/sync.md
    trunk/gcc/testsuite/ChangeLog


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=37908

[Bug middle-end/37908] atomic NAND op generate wrong code; __sync_nand_and_fetch, __sync_fetch_and_nand

[howarth at nitro dot med dot uc dot edu]

------- Comment #13 from howarth at nitro dot med dot uc dot edu  2008-11-21 17:28 -------
This test case is failing on powerpc-apple-darwin9 as follows...

Executing on host:
/sw/src/fink.build/gcc44-4.3.999-20081120/darwin_objdir/gcc/xgcc
-B/sw/src/fink.build/gcc44-4.3.999-20081120/darwin_objdir/gcc/
/sw/src/fink.build/gcc44-4.3.999-20081120/gcc-4.4-20081120/gcc/testsuite/gcc.dg/pr37908.c
  -Wsync-nand  -lm   -o ./pr37908.exe    (timeout = 300)
/sw/src/fink.build/gcc44-4.3.999-20081120/gcc-4.4-20081120/gcc/testsuite/gcc.dg/pr37908.c:
In function 'main':
/sw/src/fink.build/gcc44-4.3.999-20081120/gcc-4.4-20081120/gcc/testsuite/gcc.dg/pr37908.c:19:
note: '__sync_nand_and_fetch' changed semantics in GCC 4.4
output is:
/sw/src/fink.build/gcc44-4.3.999-20081120/gcc-4.4-20081120/gcc/testsuite/gcc.dg/pr37908.c:
In function 'main':
/sw/src/fink.build/gcc44-4.3.999-20081120/gcc-4.4-20081120/gcc/testsuite/gcc.dg/pr37908.c:19:
note: '__sync_nand_and_fetch' changed semantics in GCC 4.4

PASS: gcc.dg/pr37908.c  (test for warnings, line 19)
PASS: gcc.dg/pr37908.c (test for excess errors)
Setting LD_LIBRARY_PATH to
:/sw/src/fink.build/gcc44-4.3.999-20081120/darwin_objdir/gcc::/sw/src/fink.build/gcc44-4.3.999-20081120/darwin_objdir/gcc:/usr/local/NMRPipe/xview/mac/lib:/usr/openwin/lib:/usr/local/NMRPipe/nmrbin.mac/lib:/usr/local/lib
FAIL: gcc.dg/pr37908.c execution test


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=37908

[Bug middle-end/37908] atomic NAND op generate wrong code; __sync_nand_and_fetch, __sync_fetch_and_nand

[howarth at nitro dot med dot uc dot edu]

------- Comment #14 from howarth at nitro dot med dot uc dot edu  2008-11-21 17:29 -------
Created an attachment (id=16742)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=16742&action=view)
preprocessed file for /gcc.dg/pr37908.c on powerpc-apple-darwin9


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=37908

[Bug middle-end/37908] atomic NAND op generate wrong code; __sync_nand_and_fetch, __sync_fetch_and_nand

[howarth at nitro dot med dot uc dot edu]

------- Comment #15 from howarth at nitro dot med dot uc dot edu  2008-11-21 17:30 -------
Created an attachment (id=16743)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=16743&action=view)
assembly file for /gcc.dg/pr37908.c on powerpc-apple-darwin9


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=37908

[Bug middle-end/37908] atomic NAND op generate wrong code; __sync_nand_and_fetch, __sync_fetch_and_nand

[uros at gcc dot gnu dot org]

------- Comment #16 from uros at gcc dot gnu dot org  2008-12-01 13:50 -------
Subject: Bug 37908

Author: uros
Date: Mon Dec  1 13:48:52 2008
New Revision: 142313

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=142313
Log:
        PR middle-end/37908
        * config/alpha/alpha.c (alpha_split_atomic_op): Properly handle NAND
        case by calculating ~(new_reg & val) instead of (~new_reg & val).
        * config/alpha/sync.md (sync_nand<mode>): Change insn RTX
        to (not:I48MODE (and:I48MODE (...))).
        (sync_old_nand<mode>): Ditto.
        (sync_new_nand<mode>): Ditto.


Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/alpha/alpha.c
    trunk/gcc/config/alpha/sync.md


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=37908

[Bug middle-end/37908] atomic NAND op generate wrong code; __sync_nand_and_fetch, __sync_fetch_and_nand

[ubizjak at gmail dot com]

------- Comment #17 from ubizjak at gmail dot com  2008-12-10 09:55 -------
Fixed on 4.4 branch, WONTFIX on earlier branches.


-- 

ubizjak at gmail dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |FIXED


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=37908