[Bug c++/57926] Atomic functions broken with C++ but not C?

[Bug c++/57926] Atomic functions broken with C++ but not C?

[lailavrazda1979 at gmail dot com]

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

--- Comment #1 from lailavrazda1979 at gmail dot com ---
Created attachment 30523
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=30523&action=edit
Code triggering the bug

[Bug c++/57926] Atomic functions broken with C++ but not C?

[redi at gcc dot gnu.org]

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

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |rejects-valid
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2013-07-18
     Ever confirmed|0                           |1
           Severity|major                       |normal

--- Comment #2 from Jonathan Wakely <redi at gcc dot gnu.org> ---
The test is broken, i should be the same type as Mutex.  This should compile
though:

long Mutex[1];

int AcquireLogMutex(void)
{
    return __atomic_exchange_n(Mutex, 1, __ATOMIC_SEQ_CST);
}

void ReleaseLogMutex(void)
{
    long i = 0;
    __atomic_store(Mutex, &i, __ATOMIC_SEQ_CST);
}


G++ doesn't do the array-to-pointer conversion

There's a simple workaround: just use (long*)Mutex

[Bug c++/57926] Atomic functions broken with C++ but not C?

[paolo.carlini at oracle dot com]

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

Paolo Carlini <paolo.carlini at oracle dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |amacleod at redhat dot com

--- Comment #3 from Paolo Carlini <paolo.carlini at oracle dot com> ---
CC-ing Andrew for the array->pointer issue. Or is this a more general C++
front-end issue?

[Bug c++/57926] Atomic functions broken with C++ but not C?

[amacleod at redhat dot com]

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

--- Comment #4 from Andrew Macleod <amacleod at redhat dot com> ---
Short answer.. I'm not a sure, but it appears to be a g++ thing.

I looks like g++ is using ARRAY_TYPE instead of POINTER_TYPE.  the 4.8 branch
does the same thing, but it does seem odd to me.  That means anywhere in the
compiler that uses POINTER_TYPE_P() is not going to trigger true for an array
in c++...

Going back to gcc 4.6 which only had __sync operations, the same issue exists:

<x220:/home/compilers/gcc-4_6-branch/build/gcc>cat t.cpp
long int Mutex[1];
int AcquireLogMutex(void)
{
    return(__sync_fetch_and_add(Mutex, 1));
}


int main() {}
<x220:/home/compilers/gcc-4_6-branch/build/gcc>./cc1plus t.cpp
 int AcquireLogMutex()
t.cpp:5:38: error: incompatible type for argument 1 of ‘__sync_fetch_and_add’


Im stunned this has never been tripped over. 

I can fix it in the atomic code, but I don't know what that means to other
languages since I've never paid attention to the detailed meaning of ARRAY_TYPE
vs POINTER_TYPE.  Just allowing the ARRAY_TYPE objects to pass the existing
atomic tests of POINTER_TYPE_P or POINTER_TYPE appears to generate all the
correct code and errors for atomics, so I guess thats is the way to go...?   
there are only a couple of places the check is made.
>From gcc-bugs-return-426519-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org Mon Jul 22 16:42:44 2013
Return-Path: <gcc-bugs-return-426519-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org>
Delivered-To: listarch-gcc-bugs@gcc.gnu.org
Received: (qmail 24340 invoked by alias); 22 Jul 2013 16:42:44 -0000
Mailing-List: contact gcc-bugs-help@gcc.gnu.org; run by ezmlm
Precedence: bulk
List-Id: <gcc-bugs.gcc.gnu.org>
List-Archive: <http://gcc.gnu.org/ml/gcc-bugs/>
List-Post: <mailto:gcc-bugs@gcc.gnu.org>
List-Help: <mailto:gcc-bugs-help@gcc.gnu.org>
Sender: gcc-bugs-owner@gcc.gnu.org
Delivered-To: mailing list gcc-bugs@gcc.gnu.org
Received: (qmail 22024 invoked by uid 48); 22 Jul 2013 16:40:42 -0000
From: "pchang9 at cs dot wisc.edu" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: [Bug middle-end/57811] Wasted work in find_reloads()
Date: Mon, 22 Jul 2013 16:42:00 -0000
X-Bugzilla-Reason: CC
X-Bugzilla-Type: changed
X-Bugzilla-Watch-Reason: None
X-Bugzilla-Product: gcc
X-Bugzilla-Component: middle-end
X-Bugzilla-Version: 4.9.0
X-Bugzilla-Keywords:
X-Bugzilla-Severity: normal
X-Bugzilla-Who: pchang9 at cs dot wisc.edu
X-Bugzilla-Status: UNCONFIRMED
X-Bugzilla-Priority: P3
X-Bugzilla-Assigned-To: unassigned at gcc dot gnu.org
X-Bugzilla-Target-Milestone: ---
X-Bugzilla-Flags:
X-Bugzilla-Changed-Fields:
Message-ID: <bug-57811-4-iMGg4dYjBy@http.gcc.gnu.org/bugzilla/>
In-Reply-To: <bug-57811-4@http.gcc.gnu.org/bugzilla/>
References: <bug-57811-4@http.gcc.gnu.org/bugzilla/>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
X-Bugzilla-URL: http://gcc.gnu.org/bugzilla/
Auto-Submitted: auto-generated
MIME-Version: 1.0
X-SW-Source: 2013-07/txt/msg01026.txt.bz2
Content-length: 203

http://gcc.gnu.org/bugzilla/show_bug.cgi?idW811

--- Comment #4 from Po-Chun Chang <pchang9 at cs dot wisc.edu> ---
Patch sent to gcc-patches@

http://gcc.gnu.org/ml/gcc-patches/2013-07/msg00930.html

[Bug c++/57926] Atomic functions broken with C++ but not C?

[paolo.carlini at oracle dot com]

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

Paolo Carlini <paolo.carlini at oracle dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jason at gcc dot gnu.org

--- Comment #5 from Paolo Carlini <paolo.carlini at oracle dot com> ---
Thanks a lot Andrew. I think it makes sense to CC Jason.

[Bug c++/57926] Atomic functions broken with C++ but not C?

[amacleod at redhat dot com]

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

Andrew Macleod <amacleod at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at gcc dot gnu.org      |amacleod at redhat dot com

--- Comment #6 from Andrew Macleod <amacleod at redhat dot com> ---
Created attachment 30535
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=30535&action=edit
should fix bug

So this would appear to fix the problem by allowing ARRAY_TYPE as well as
POINTER_TYPE to appear where the atomic pointer is required.

I haven't bootstrapped it or anything yet, but assuming that ARRAY_TYPE from
the g++ front-end means the same as POINTER_TYPE in this context, this would be
the solution and I'll proceed with a test case and proper bootstrap/submission.

You can try it if you want :-) I tried a few different contexts and got the
behaviour I expected in each case.

[Bug c++/57926] Atomic functions broken with C++ but not C?

[amacleod at redhat dot com]

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

--- Comment #7 from Andrew Macleod <amacleod at redhat dot com> ---
btw, that patch passes bootstrap and a new testcase based on a modified version
of the bug report test.

[Bug c++/57926] Atomic functions broken with C++ but not C?

[amacleod at redhat dot com]

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

Andrew Macleod <amacleod at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |rth at gcc dot gnu.org

--- Comment #9 from Andrew Macleod <amacleod at redhat dot com> ---
I don't know either.. that parameter checking stuff is all inherited from the
original __sync code.

resolve_overloaded_builtin() is called avery early and it takes care of
everything, telling the call handling code to not do anything else.  

In C it would have to call array_to_pointer_conversion() for arrays,  and
decay_conversion for c++..  ugg. that means it needs to be done *before* the
overloaded resolution. Thats pretty ugly.

Im going to copy rth since he wrote this code originally.  Maybe he knows the
easy shortcut :-)


Andrew

[Bug c++/57926] Atomic functions broken with C++ but not C?

[lailavrazda1979 at gmail dot com]

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

--- Comment #11 from lailavrazda1979 at gmail dot com ---
I don't mean to be a bother, but this hasn't been updated in a while. Has it
been fixed?

[Bug c++/57926] Atomic functions broken with C++ but not C?

[lailavrazda1979 at gmail dot com]

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

--- Comment #12 from lailavrazda1979 at gmail dot com ---
Bug still a problem with latest trunk.

[Bug c++/57926] Atomic functions broken with C++ but not C?

[jason at gcc dot gnu.org]

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

Jason Merrill <jason at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|amacleod at redhat dot com         |jason at gcc dot gnu.org

[Bug c++/57926] Atomic functions broken with C++ but not C?

[jason at gcc dot gnu.org]

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

--- Comment #13 from Jason Merrill <jason at gcc dot gnu.org> ---
Created attachment 32575
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=32575&action=edit
patch

This patch forces the decay for C++.  We don't need to do anything for C, since
arrays decay immediately when named in the C front end.  I think I'm inclined
to wait until after 4.9 to check this in.

[Bug c++/57926] Atomic functions broken with C++ but not C?

[lailavrazda1979 at gmail dot com]

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

--- Comment #14 from lailavrazda1979 at gmail dot com ---
Why wait? I'm not hugely opposed, but bugfixes are bugfixes, and one more fixed
bug makes a better 4.9 release, right?

[Bug c++/57926] Atomic functions broken with C++ but not C?

[jason at gcc dot gnu.org]

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

--- Comment #15 from Jason Merrill <jason at gcc dot gnu.org> ---
(In reply to lailavrazda1979 from comment #14)
> Why wait? I'm not hugely opposed, but bugfixes are bugfixes, and one more
> fixed bug makes a better 4.9 release, right?

Because all changes risk introducing new bugs, and we're very close to 4.9 now.

[Bug c++/57926] Atomic functions broken with C++ but not C?

[lailavrazda1979 at gmail dot com]

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

--- Comment #16 from lailavrazda1979 at gmail dot com ---
Okay, no worries.

[Bug c++/57926] Atomic functions broken with C++ but not C?

[jason at gcc dot gnu.org]

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

--- Comment #17 from Jason Merrill <jason at gcc dot gnu.org> ---
Author: jason
Date: Fri Apr 11 18:25:13 2014
New Revision: 209316

URL: http://gcc.gnu.org/viewcvs?rev=209316&root=gcc&view=rev
Log:
    PR c++/57926
    * c-common.c (sync_resolve_size, get_atomic_generic_size): Call
    default_conversion for an array argument.

Added:
    trunk/gcc/testsuite/g++.dg/ext/atomic-2.C
Modified:
    trunk/gcc/c-family/ChangeLog
    trunk/gcc/c-family/c-common.c

[Bug c++/57926] Atomic functions broken with C++ but not C?

[lailavrazda1979 at gmail dot com]

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

lailavrazda1979 at gmail dot com changed:

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

--- Comment #18 from lailavrazda1979 at gmail dot com ---
Patch is checked in, so I'm closing this.