Bug 12496 - wrong result for __atomic_add(&value, -1) when using -O0 -m64
Summary: wrong result for __atomic_add(&value, -1) when using -O0 -m64
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: libstdc++ (show other bugs)
Version: 3.2.3
: P2 normal
Target Milestone: 3.3.3
Assignee: David S. Miller
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2003-10-03 07:30 UTC by patrick.frants
Modified: 2003-12-09 05:58 UTC (History)
1 user (show)

See Also:
Host: sparc-sun-solaris2.8
Target: sparc-sun-solaris2.8
Build: sparc-sun-solaris2.8
Known to work:
Known to fail:
Last reconfirmed: 2003-10-28 09:49:11


Attachments
Fix atomic add/dec sign extension on 64-bit sparc. (334 bytes, patch)
2003-12-06 01:21 UTC, David S. Miller
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description patrick.frants 2003-10-03 07:30:54 UTC
First of all I don't know wether it is a library or code generation problem, as 
I don't know wether the inline assembler in atomicity.h *should* produce 
correct code.

With -o0 __atomic_add with -1 as the second argument results in 4294967305. 
With all other optimization levels it results in the right value.

The output of the program below is:

bash-2.03$ g++ -m64 atomic_add_bug.cpp 
bash-2.03$ a.out
__atomic_add(10,  1) = 11
__atomic_add(10, -1) = 4294967305


/* Compile this on Sparc:
 * "g++ -o0 -m64 atomic_add_bug.cpp"
 * Execute a.out and see that the output for adding -1 is incorrect!
 *
 * Repeating the same procedure for -o1, -o2, -o3 shows that those
 * optimization levels produce expected output.
 */

#include <iostream>
using namespace std;

#include <bits/atomicity.h>

int main(int argc, char** argv)
{
  _Atomic_word inc_result = 10;
  _Atomic_word dec_result = 10;

  __atomic_add(&inc_result, 1);
  __atomic_add(&dec_result, -1);

  cout << "__atomic_add(10,  1) = " << inc_result << endl;
  cout << "__atomic_add(10, -1) = " << dec_result << endl;
  return 0;
}
Comment 1 Eric Botcazou 2003-10-28 09:49:11 UTC
Confirmed with GCC 3.2.3 and GCC 3.3.2 at least.
Comment 2 Benjamin Kosnik 2003-12-04 19:40:37 UTC
Any chance you could look at this?

thanks,
benjamin
Comment 3 David S. Miller 2003-12-06 01:10:36 UTC
Notice that the value 4294967305 is the expected value ("9") with bit
32 also set.  Smells like a missing sign extension from 32 to 64 bits
somewhere.

I'll look more deeply into this.
Comment 4 David S. Miller 2003-12-06 01:21:07 UTC
The bug is that the __atomic_{inc,dec} routines were not properly
sign extending the __val argument from an int to an _Atomic_word.

I'm going to attach a patch I am testing right now.
Comment 5 David S. Miller 2003-12-06 01:21:50 UTC
Created attachment 5287 [details]
Fix atomic add/dec sign extension on 64-bit sparc.
Comment 6 David S. Miller 2003-12-06 01:23:10 UTC
As a side note, this bug explains some of the libjava testsuite failures
we get on 64-bit sparc which I have been stumped about for some time.
Comment 7 Paolo Carlini 2003-12-08 09:44:35 UTC
Hi,

from a coding conventions point of view, it would be better if you could prefer
names starting with double underscore.

Thanks.
Comment 8 GCC Commits 2003-12-08 10:01:06 UTC
Subject: Bug 12496

CVSROOT:	/cvs/gcc
Module name:	gcc
Changes by:	davem@gcc.gnu.org	2003-12-08 10:01:02

Modified files:
	libstdc++-v3   : ChangeLog 
	libstdc++-v3/config/cpu/sparc: atomicity.h 

Log message:
	2003-12-08  David S. Miller  <davem@redhat.com>
	
	PR libstdc++/12496
	* config/cpu/sparc/atomicity.h (__exchange_and_add, __atomic_add):
	Extend increment to _Atomic_word before giving to assembler.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/ChangeLog.diff?cvsroot=gcc&r1=1.2133&r2=1.2134
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/config/cpu/sparc/atomicity.h.diff?cvsroot=gcc&r1=1.2&r2=1.3

Comment 9 David S. Miller 2003-12-08 10:06:40 UTC
Fixed in mainline.
Comment 10 GCC Commits 2003-12-09 05:44:12 UTC
Subject: Bug 12496

CVSROOT:	/cvs/gcc
Module name:	gcc
Branch: 	gcc-3_3-branch
Changes by:	davem@gcc.gnu.org	2003-12-09 05:44:01

Modified files:
	libstdc++-v3   : ChangeLog 
	libstdc++-v3/config/cpu/sparc: atomicity.h 

Log message:
	2003-12-08  David S. Miller  <davem@redhat.com>
	
	PR libstdc++/12496
	* config/cpu/sparc/atomicity.h (__exchange_and_add, __atomic_add):
	Extend increment to _Atomic_word before giving to assembler.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-3_3-branch&r1=1.1464.2.158&r2=1.1464.2.159
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/config/cpu/sparc/atomicity.h.diff?cvsroot=gcc&only_with_tag=gcc-3_3-branch&r1=1.1&r2=1.1.20.1