Re: [PATCH, libstdc++] Improve slightly __cxa_guard_acquire
On Thu, Sep 06, 2012 at 11:10:37PM +0200, Jakub Jelinek wrote:
+ int expected(0);
if (__atomic_compare_exchange_n(gi, expected, pending_bit, false,
__ATOMIC_ACQ_REL,
__ATOMIC_RELAXED))
Shouldn't this __ATOMIC_RELAXED be also __ATOMIC_ACQUIRE? If expected ends
up being guard_bit, then the code will return 0; right away.
Here is a patch for that. Ok for trunk/4.7?
2012-09-11 Jakub Jelinek ja...@redhat.com
PR libstdc++/54172
* libsupc++/guard.cc (__cxa_guard_acquire): Fix up the last
argument of the first __atomic_compare_exchange_n.
--- libstdc++-v3/libsupc++/guard.cc.jj 2012-09-11 16:55:16.0 +0200
+++ libstdc++-v3/libsupc++/guard.cc 2012-09-11 16:56:38.035848876 +0200
@@ -253,7 +253,7 @@ namespace __cxxabiv1
int expected(0);
if (__atomic_compare_exchange_n(gi, expected, pending_bit, false,
__ATOMIC_ACQ_REL,
- __ATOMIC_RELAXED))
+ __ATOMIC_ACQUIRE))
{
// This thread should do the initialization.
return 1;
Jakub
Re: [PATCH, libstdc++] Improve slightly __cxa_guard_acquire
On 09/11/2012 08:02 AM, Jakub Jelinek wrote: 2012-09-11 Jakub Jelinek ja...@redhat.com PR libstdc++/54172 * libsupc++/guard.cc (__cxa_guard_acquire): Fix up the last argument of the first __atomic_compare_exchange_n. Looks good. r~
Re: [PATCH, libstdc++] Improve slightly __cxa_guard_acquire
On Thu, 30 Aug 2012 12:48:34 +0200 Thiago Macieira thiago.macie...@intel.com wrote: Hello The attached patch is a simple improvement to make a thread that failed to set the waiting bit to exit the function earlier, if it detects that another thread has successfully finished initialising. It matches the CAS code from a few lines above. The change from RELAXED to ACQUIRE is noted in the previous patch I've just sent. I like this, but want // make a thread that failed to set the waiting bit to exit the function // earlier, if it detects that another thread has successfully finished // initialising added as a comment in the if (expected == pending_bit) branch. I would like to put this in trunk + comment and give it 2-3 days at least before 4.7 branch. -benjamin
Re: [PATCH, libstdc++] Improve slightly __cxa_guard_acquire
On quinta-feira, 6 de setembro de 2012 13.33.11, Benjamin De Kosnik wrote: Here's the patch as applied to trunk in rev. 191042. I'll apply it to 4.7 this weekend as long as nobody yelps. Thanks. The change to ACQUIRE is also a bugfix. -- Thiago Macieira - thiago.macieira (AT) intel.com Software Architect - Intel Open Source Technology Center Intel Sweden AB - Registration Number: 556189-6027 Knarrarnäsgatan 15, 164 40 Kista, Stockholm, Sweden signature.asc Description: This is a digitally signed message part.
Re: [PATCH, libstdc++] Improve slightly __cxa_guard_acquire
On Thu, Sep 06, 2012 at 01:33:11PM -0700, Benjamin De Kosnik wrote:
Here's the patch as applied to trunk in rev. 191042. I'll apply it to
4.7 this weekend as long as nobody yelps.
2012-09-06 Thiago Macieira thiago.macie...@intel.com
PR libstdc++/54172
* libsupc++/guard.cc (__cxa_guard_acquire): Exit the loop earlier if
we detect that another thread has had success. Don't compare_exchange
from a finished state back to a waiting state. Comment.
diff --git a/libstdc++-v3/libsupc++/guard.cc b/libstdc++-v3/libsupc++/guard.cc
index adc9608..60165cd 100644
--- a/libstdc++-v3/libsupc++/guard.cc
+++ b/libstdc++-v3/libsupc++/guard.cc
@@ -244,13 +244,13 @@ namespace __cxxabiv1
if (__gthread_active_p ())
{
int *gi = (int *) (void *) g;
- int expected(0);
const int guard_bit = _GLIBCXX_GUARD_BIT;
const int pending_bit = _GLIBCXX_GUARD_PENDING_BIT;
const int waiting_bit = _GLIBCXX_GUARD_WAITING_BIT;
while (1)
{
+ int expected(0);
if (__atomic_compare_exchange_n(gi, expected, pending_bit, false,
__ATOMIC_ACQ_REL,
__ATOMIC_RELAXED))
Shouldn't this __ATOMIC_RELAXED be also __ATOMIC_ACQUIRE? If expected ends
up being guard_bit, then the code will return 0; right away.
@@ -264,13 +264,26 @@ namespace __cxxabiv1
// Already initialized.
return 0;
}
+
if (expected == pending_bit)
{
+ // Use acquire here.
int newv = expected | waiting_bit;
if (!__atomic_compare_exchange_n(gi, expected, newv, false,
__ATOMIC_ACQ_REL,
- __ATOMIC_RELAXED))
-continue;
+ __ATOMIC_ACQUIRE))
+{
+ if (expected == guard_bit)
+{
+ // Make a thread that failed to set the
+ // waiting bit exit the function earlier,
+ // if it detects that another thread has
+ // successfully finished initialising.
+ return 0;
+}
+ if (expected == 0)
+continue;
+}
expected = newv;
}
Jakub
[PATCH, libstdc++] Improve slightly __cxa_guard_acquire
Hello
The attached patch is a simple improvement to make a thread that failed to set
the waiting bit to exit the function earlier, if it detects that another
thread has successfully finished initialising. It matches the CAS code from a
few lines above.
The change from RELAXED to ACQUIRE is noted in the previous patch I've just
sent.
--
Thiago Macieira - thiago.macieira (AT) intel.com
Software Architect - Intel Open Source Technology Center
Intel Sweden AB - Registration Number: 556189-6027
Knarrarnäsgatan 15, 164 40 Kista, Stockholm, Sweden
2012-08-30 Thiago Macieira thiago.macie...@intel.com
* libsupc++/guard.cc (__cxa_guard_acquire): exit the loop earlier if
we detect that another thread has had success.
---
libstdc++-v3/libsupc++/guard.cc | 12 ++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/libstdc++-v3/libsupc++/guard.cc b/libstdc++-v3/libsupc++/guard.cc
index bfe1a59..73d7221 100644
--- a/libstdc++-v3/libsupc++/guard.cc
+++ b/libstdc++-v3/libsupc++/guard.cc
@@ -269,8 +269,16 @@ namespace __cxxabiv1
int newv = expected | waiting_bit;
if (!__atomic_compare_exchange_n(gi, expected, newv, false,
__ATOMIC_ACQ_REL,
- __ATOMIC_RELAXED))
- continue;
+ __ATOMIC_ACQUIRE))
+ {
+ if (expected == guard_bit)
+ {
+ // Already initialized.
+ return 0;
+ }
+ if (expected == 0)
+ continue;
+ }
expected = newv;
}
--
1.7.11.4
