[Bug target/24757] [4.1 Regression] __sync_fetch_and_add on ia64
--- Comment #33 from schwab at gcc dot gnu dot org 2005-11-20 10:43 --- Subject: Bug 24757 Author: schwab Date: Sun Nov 20 10:43:43 2005 New Revision: 107246 URL: http://gcc.gnu.org/viewcvs?root=gccview=revrev=107246 Log: PR target/24757 * config/ia64/ia64.c (ia64_expand_atomic_op): Fix condition of cmp insn. Modified: trunk/gcc/ChangeLog trunk/gcc/config/ia64/ia64.c -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=24757
[Bug target/24757] [4.1 Regression] __sync_fetch_and_add on ia64
--- Comment #34 from schwab at gcc dot gnu dot org 2005-11-20 10:44 --- Subject: Bug 24757 Author: schwab Date: Sun Nov 20 10:44:27 2005 New Revision: 107247 URL: http://gcc.gnu.org/viewcvs?root=gccview=revrev=107247 Log: PR target/24757 * config/ia64/ia64.c (ia64_expand_atomic_op): Fix condition of cmp insn. Modified: branches/gcc-4_1-branch/gcc/ChangeLog branches/gcc-4_1-branch/gcc/config/ia64/ia64.c -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=24757
[Bug target/24757] [4.1 Regression] __sync_fetch_and_add on ia64
--- Comment #35 from schwab at suse dot de 2005-11-20 10:45 --- Fixed. -- schwab at suse dot de changed: What|Removed |Added Status|ASSIGNED|RESOLVED Resolution||FIXED http://gcc.gnu.org/bugzilla/show_bug.cgi?id=24757
[Bug target/24757] [4.1 Regression] __sync_fetch_and_add on ia64
--- Comment #26 from pcarlini at suse dot de 2005-11-19 22:12 --- Created an attachment (id=10297) -- (http://gcc.gnu.org/bugzilla/attachment.cgi?id=10297action=view) Reduced from thread/pthread3 Richard, I'm attaching a really minimal testcase, which fails very quickly for me with mainline or 4_1-branch and doesn't with 4_0-branch. It's just two threads and many constructions and destructions of a trivial class which increases (descreases, respectively) atomically a static counter. Of course the abort should never trigger. Can you debug further with this? Otherwise, just tell me, ok? -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=24757
[Bug target/24757] [4.1 Regression] __sync_fetch_and_add on ia64
--- Comment #27 from rguenth at gcc dot gnu dot org 2005-11-19 22:38 --- The only thing in the pthread3_reduced.cc testcase that could make it going wrong is wrong alignment of aw. And I remember seeing a lot of unaligned traps in dmesg during libstdc++ testing on ia64. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=24757
[Bug target/24757] [4.1 Regression] __sync_fetch_and_add on ia64
--- Comment #28 from pcarlini at suse dot de 2005-11-19 22:45 --- Thanks Richard G. In fact, from time to time I saw warnings on the shell, which really puzzled me, because I was sure that no alignment issues were present anymore in the higher lever library code proper. I'll try to add an alignment attribute to the _Atomic_word typedef, as cris is already doing for instance, and see whether something changes for the better. Interestingly no such issue existed with 4_0-branch... Any idea why? -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=24757
[Bug target/24757] [4.1 Regression] __sync_fetch_and_add on ia64
--- Comment #29 from pcarlini at suse dot de 2005-11-19 23:02 --- (In reply to comment #28) I'll try to add an alignment attribute to the _Atomic_word typedef, as cris is already doing for instance, and see whether something changes for the better. Nope. Maybe the alignment is for some reason broken to the point that not even an explicit __attribute__ ((aligned (X))) can fix it... I don't know. Anyway, the problem is very easy to reproduce, now. I'm also attaching a further reduced, pure C testcase. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=24757
[Bug target/24757] [4.1 Regression] __sync_fetch_and_add on ia64
--- Comment #30 from pcarlini at suse dot de 2005-11-19 23:06 --- Created an attachment (id=10298) -- (http://gcc.gnu.org/bugzilla/attachment.cgi?id=10298action=view) Further reduced, pure C -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=24757
[Bug target/24757] [4.1 Regression] __sync_fetch_and_add on ia64
--- Comment #31 from schwab at suse dot de 2005-11-20 00:22 --- Testing this patch: Index: ia64.c === --- ia64.c (revision 107220) +++ ia64.c (working copy) @@ -2113,7 +2113,7 @@ emit_insn (GEN_FCN (icode) (cmp_reg, mem, ar_ccv, new_reg)); - emit_cmp_and_jump_insns (cmp_reg, old_reg, EQ, NULL, DImode, true, label); + emit_cmp_and_jump_insns (cmp_reg, old_reg, NE, NULL, DImode, true, label); } /* Begin the assembly file. */ -- schwab at suse dot de changed: What|Removed |Added AssignedTo|unassigned at gcc dot gnu |schwab at suse dot de |dot org | Status|NEW |ASSIGNED http://gcc.gnu.org/bugzilla/show_bug.cgi?id=24757
[Bug target/24757] [4.1 Regression] __sync_fetch_and_add on ia64
--- Comment #32 from rth at gcc dot gnu dot org 2005-11-20 05:43 --- Oh good grief. I can't see the forest for the trees. Andreas, I expect that patch will work. If it tests ok, please commit it. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=24757
[Bug target/24757] [4.1 Regression] __sync_fetch_and_add on ia64
--- Comment #9 from pinskia at gcc dot gnu dot org 2005-11-18 15:20 --- I should note that ld.acq+cmpxchg.rel is a full barrier. as noted in ia64.c so a mf is not required. See also http://www.ussg.iu.edu/hypermail/linux/kernel/0205.1/0226.html -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=24757
[Bug target/24757] [4.1 Regression] __sync_fetch_and_add on ia64
--- Comment #10 from pcarlini at suse dot de 2005-11-18 15:23 --- (In reply to comment #9) I should note that ld.acq+cmpxchg.rel is a full barrier. as noted in ia64.c so a mf is not required. Did you read the last message from Alexander on the libstdc++ mailing list, in particular the last paragraph? -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=24757
[Bug target/24757] [4.1 Regression] __sync_fetch_and_add on ia64
--- Comment #11 from pinskia at gcc dot gnu dot org 2005-11-18 15:24 --- (In reply to comment #9) I should note that ld.acq+cmpxchg.rel is a full barrier. as noted in ia64.c so a mf is not required. If that is really the issue, then http://gcc.gnu.org/ml/gcc-patches/2005-05/msg00872.html casued it but I really doubt it from reading that linux kernel message. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=24757
[Bug target/24757] [4.1 Regression] __sync_fetch_and_add on ia64
--- Comment #12 from pinskia at gcc dot gnu dot org 2005-11-18 15:27 --- (In reply to comment #10) Did you read the last message from Alexander on the libstdc++ mailing list, in particular the last paragraph? Yes but I trust RTH better than Alexander on this issue. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=24757
[Bug target/24757] [4.1 Regression] __sync_fetch_and_add on ia64
--- Comment #13 from pinskia at gcc dot gnu dot org 2005-11-18 15:29 --- (In reply to comment #12) Yes but I trust RTH better than Alexander on this issue. And if you read the message which I pointed to: The semaphore instructions also have implicit ordering. So if I am reading this wrong, then there is something else wrong. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=24757
[Bug target/24757] [4.1 Regression] __sync_fetch_and_add on ia64
--- Comment #14 from pcarlini at suse dot de 2005-11-18 15:32 --- I'm not sure that specific patch is the culprit, because (see Comment #5) the threaded testcases started failing before that date. I would like to see the assembly before and after that date. In any case, it would be nice if Richard Henderson could analyze a bit this PR. Also relevant: http://gcc.gnu.org/ml/libstdc++/2005-11/msg00206.html http://gcc.gnu.org/ml/libstdc++/2005-11/msg00207.html -- pcarlini at suse dot de changed: What|Removed |Added CC||rth at gcc dot gnu dot org http://gcc.gnu.org/bugzilla/show_bug.cgi?id=24757
[Bug target/24757] [4.1 Regression] __sync_fetch_and_add on ia64
--- Comment #15 from pinskia at gcc dot gnu dot org 2005-11-18 15:35 --- See also: http://www.linuxbase.org/spec/refspecs/IA64-softdevman-vol1.pdf Page73. Which is where the comment in comment #13. So If I read table 4-20 currently then the memory access will always be ordered in that loop. And if it started before that date, then there is something wrong as that is when the major difference between 4.0 and 4.1 started. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=24757
[Bug target/24757] [4.1 Regression] __sync_fetch_and_add on ia64
--- Comment #16 from rth at gcc dot gnu dot org 2005-11-18 23:32 --- Alex's explanation can't be all of it. I hacked the compiler to emit two mf instructions before and after the sequence here, and it made no difference to the two tests failing. I really have no idea what's going on. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=24757
[Bug target/24757] [4.1 Regression] __sync_fetch_and_add on ia64
--- Comment #17 from pcarlini at suse dot de 2005-11-18 23:41 --- (In reply to comment #16) Alex's explanation can't be all of it. I hacked the compiler to emit two mf instructions before and after the sequence here, and it made no difference to the two tests failing. I really have no idea what's going on. Thanks for your experiments. I'll try to help further, come up with a reduced piece of code actually failing, investigate whether something else important went in mid of April, maybe. I don't have much more, right now :( -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=24757
[Bug target/24757] [4.1 Regression] __sync_fetch_and_add on ia64
--- Comment #18 from rth at gcc dot gnu dot org 2005-11-19 00:56 --- Dammit, lemme try again; I patched a different tree than I tested. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=24757
[Bug target/24757] [4.1 Regression] __sync_fetch_and_add on ia64
--- Comment #19 from pcarlini at suse dot de 2005-11-19 01:05 --- Created an attachment (id=10287) -- (http://gcc.gnu.org/bugzilla/attachment.cgi?id=10287action=view) Slightly hacked, self contained test (for use outside the testsuite) -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=24757
[Bug target/24757] [4.1 Regression] __sync_fetch_and_add on ia64
--- Comment #20 from pcarlini at suse dot de 2005-11-19 01:06 --- Created an attachment (id=10288) -- (http://gcc.gnu.org/bugzilla/attachment.cgi?id=10288action=view) Slightly hacked, self contained test (for use outside the testsuite) -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=24757
[Bug target/24757] [4.1 Regression] __sync_fetch_and_add on ia64
--- Comment #21 from pcarlini at suse dot de 2005-11-19 01:08 --- Just in case, those are ready to use outside the testsuite... For me, especially the second one, fails very quickly and consistently on 4-way and 16-way. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=24757
[Bug target/24757] [4.1 Regression] __sync_fetch_and_add on ia64
--- Comment #22 from mmitchel at gcc dot gnu dot org 2005-11-19 02:08 --- I think we need to understand this problem before we release. It might be that we downgrade the priority if it turns out to be a problem outside our control. -- mmitchel at gcc dot gnu dot org changed: What|Removed |Added Priority|P3 |P1 http://gcc.gnu.org/bugzilla/show_bug.cgi?id=24757
[Bug target/24757] [4.1 Regression] __sync_fetch_and_add on ia64
--- Comment #23 from rth at gcc dot gnu dot org 2005-11-19 02:09 --- Created an attachment (id=10289) -- (http://gcc.gnu.org/bugzilla/attachment.cgi?id=10289action=view) mf hack For the record, here's the aforementioned hack. And it does appear to change the behaviour of one of the testsuite tests, but it doesn't fix both of them. I'll claim, then, that it didn't fix either of them, but merely changed the timing. Feel free to tell me what's still not right, but it seems like, with a hammer this big, that it isn't the synchronization that's the problem. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=24757
[Bug target/24757] [4.1 Regression] __sync_fetch_and_add on ia64
--- Comment #24 from pcarlini at suse dot de 2005-11-19 02:15 --- (In reply to comment #23) Created an attachment (id=10289) -- (http://gcc.gnu.org/bugzilla/attachment.cgi?id=10289action=view) [edit] mf hack For the record, here's the aforementioned hack. Ok, thanks. Over the WE I'll play a bit with it. Among other things, I want to check its effects on other tests, like the thread/pthread*.cc, and also for a different version of the library, which uses more refcounting in the string class and for me exposed many more troubles during the last days. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=24757
[Bug target/24757] [4.1 Regression] __sync_fetch_and_add on ia64
--- Comment #8 from pcarlini at suse dot de 2005-11-17 13:50 ---
I have got additional evidence that __sync_fetch_and_add does not work
correctly. If, for example, in this code, stressed in the testcase
12658_thread-1.cc, and using atomic operations:
const locale
locale::operator=(const locale __other) throw()
{
__other._M_impl-_M_add_reference();
_M_impl-_M_remove_reference();
_M_impl = __other._M_impl;
return *this;
}
I wrap everything in the locale_cons mutex then the testcase doesn't fail
anymore.
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=24757
[Bug target/24757] [4.1 Regression] __sync_fetch_and_add on ia64
--- Comment #7 from pcarlini at suse dot de 2005-11-10 09:59 --- I'm marking this as a 4.1 Regression: certainly it is. If people can figure a way to workaround it at the library level, I'm ok with that. -- pcarlini at suse dot de changed: What|Removed |Added Severity|normal |major Status|UNCONFIRMED |NEW Ever Confirmed|0 |1 Last reconfirmed|-00-00 00:00:00 |2005-11-10 09:59:06 date|| Summary|__sync_fetch_and_add on ia64|[4.1 Regression] ||__sync_fetch_and_add on ia64 Target Milestone|--- |4.1.0 http://gcc.gnu.org/bugzilla/show_bug.cgi?id=24757
