[Bug regression/37640] New: Misuse of __sync_lock_test_and_set causes ICE

[Bug regression/37640] New: Misuse of __sync_lock_test_and_set causes ICE

[anton at samba dot org]

A gcc build from today (gcc version 4.4.0 20080924) gets an ICE on PowerPC when
building this (admittedly broken) code:

# gcc -m64 -O2 -c test.c

struct foo {
        int a;
        char lock;
};
struct foo *foo;

void testcase()
{
        __sync_lock_test_and_set(&(foo->lock), 0);
}

It compiles if building 32bit. Even so, I wonder if gcc should warn when the
variable is incompatible with __sync_* builtins.


-- 
           Summary: Misuse of __sync_lock_test_and_set causes ICE
           Product: gcc
           Version: 4.4.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: regression
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: anton at samba dot org
GCC target triplet: powerpc-linux


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

[Bug target/37640] Misuse of __sync_lock_test_and_set causes ICE

[pinskia at gcc dot gnu dot org]

------- Comment #1 from pinskia at gcc dot gnu dot org  2008-09-24 19:55 -------
How is this broken code?


-- 

pinskia at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|regression                  |target


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

[Bug target/37640] Misuse of __sync_lock_test_and_set causes ICE

[anton at samba dot org]

------- Comment #2 from anton at samba dot org  2008-09-24 20:07 -------
After reading the gcc documentation I guess it is valid, and the 32bit
lwarx/stwcx will overlap but not change surrounding memory.


-- 


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

[Bug target/37640] Misuse of __sync_lock_test_and_set causes ICE

[pinskia at gcc dot gnu dot org]

------- Comment #3 from pinskia at gcc dot gnu dot org  2008-09-24 20:16 -------
Something like this fixes the issue:
Index: config/rs6000/rs6000.c
===================================================================
--- config/rs6000/rs6000.c      (revision 140638)
+++ config/rs6000/rs6000.c      (working copy)
@@ -14082,7 +14082,8 @@ rs6000_expand_compare_and_swapqhi (rtx d
   HOST_WIDE_INT imask = GET_MODE_MASK (mode);

   /* Shift amount for subword relative to aligned word.  */
-  addrSI = force_reg (SImode, gen_lowpart_common (SImode, XEXP (mem, 0)));
+  addrSI = force_reg (GET_MODE (XEXP (mem, 0)), XEXP (mem, 0));
+  addrSI = force_reg (SImode, gen_lowpart_common (SImode, addrSI));
   shift = gen_reg_rtx (SImode);
   emit_insn (gen_rlwinm (shift, addrSI, GEN_INT (3),
                         GEN_INT (shift_mask)));

--- CUT ---
gen_lowpart_common does not work on non regs so force the addition to memory
first and then get the lowerpart.


-- 

pinskia at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|__sync_lock_test_and_set    |Misuse of
                   |causes ICE                  |__sync_lock_test_and_set
                   |                            |causes ICE


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

[Bug target/37640] __sync_lock_test_and_set on PPC64 causes ICE

[pinskia at gcc dot gnu dot org]

------- Comment #4 from pinskia at gcc dot gnu dot org  2008-09-28 21:17 -------
Mine, as far as I can tell this never worked.


-- 

pinskia at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |pinskia at gcc dot gnu dot
                   |                            |org
         AssignedTo|unassigned at gcc dot gnu   |pinskia at gcc dot gnu dot
                   |dot org                     |org
             Status|UNCONFIRMED                 |ASSIGNED
     Ever Confirmed|0                           |1
           Keywords|                            |ice-on-valid-code
      Known to fail|                            |4.3.0 4.4.0
   Last reconfirmed|0000-00-00 00:00:00         |2008-09-28 21:17:12
               date|                            |


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

[Bug target/37640] __sync_lock_test_and_set on PPC64 causes ICE

[pinskia at gcc dot gnu dot org]

------- Comment #5 from pinskia at gcc dot gnu dot org  2008-09-28 23:07 -------
Fixed on the trunk.


-- 

pinskia at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |FIXED
   Target Milestone|---                         |4.4.0


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

[Bug target/37640] __sync_lock_test_and_set on PPC64 causes ICE

[pinskia at gcc dot gnu dot org]

------- Comment #6 from pinskia at gcc dot gnu dot org  2008-09-28 23:08 -------
Subject: Bug 37640

Author: pinskia
Date: Sun Sep 28 23:07:01 2008
New Revision: 140740

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=140740
Log:
2008-09-28  Andrew Pinski  <andrew_pinski@playstation.sony.com>

        PR target/37640
        * config/rs6000/rs6000.c (rs6000_expand_compare_and_swapqhi): Force
        address to a register before taking the lower part.

2008-09-28  Andrew Pinski  <andrew_pinski@playstation.sony.com>

        PR target/37640
        * gcc.c-torture/compile/sync-3.c: New testcase to check that
        addresses of non zero offset works.


Added:
    trunk/gcc/testsuite/gcc.c-torture/compile/sync-3.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/rs6000/rs6000.c
    trunk/gcc/testsuite/ChangeLog


-- 


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

[Bug target/37640] __sync_lock_test_and_set on PPC64 causes ICE

[bje at gcc dot gnu dot org]

------- Comment #7 from bje at gcc dot gnu dot org  2008-11-18 22:09 -------
Subject: Bug 37640

Author: bje
Date: Tue Nov 18 22:07:58 2008
New Revision: 141980

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=141980
Log:
        Backport from mainline:

        PR target/37640
        2008-09-28  Andrew Pinski  <andrew_pinski@playstation.sony.com>

        * config/rs6000/rs6000.c (rs6000_expand_compare_and_swapqhi): Force
        address to a register before taking the lower part.

testsuite/
        * gcc.c-torture/compile/sync-3.c: New testcase to check that
        addresses of non zero offset works.

Added:
    branches/gcc-4_3-branch/gcc/testsuite/gcc.c-torture/compile/sync-3.c
Modified:
    branches/gcc-4_3-branch/gcc/ChangeLog
    branches/gcc-4_3-branch/gcc/config/rs6000/rs6000.c
    branches/gcc-4_3-branch/gcc/testsuite/ChangeLog


-- 


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