New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Increment on volatile variables in deprecated #588
Comments
Not sure the two step replacement is still 'atomic', dependent upon what assembly is generated with the double evaluation. We abused volatile because there was no good C99 way of saying its an atomic variable and the compiler shouldn't reorder around it, even if we are not doing fetch_and_op on it. In C11 there are all the good types we need, and requiring c11 is not unreasonable nowadays. There is a difference between ((volatile int)v)++ and ((atomic int)a)++: the later will substitute for a fetch_and_add automatically, the former will only enforce that no reordering and register reload is done but will use a normal ADD assembly. That may have performance implications, whether that matters is to be seen. |
The current version is not atomic either. I assume it doesn't have to be because it is used in
Agreed. BUT: since PaRSEC interfaces with C++ (TTG) and those headers are used publicly the |
Accepting
Anyway, the following patch should make the C++ compiler quiet. diff --git a/parsec/class/list_item.h b/parsec/class/list_item.h
index 0e82ac12f..cc94780f2 100644
--- a/parsec/class/list_item.h
+++ b/parsec/class/list_item.h
@@ -109,7 +109,7 @@ parsec_list_item_ring( parsec_list_item_t* first, parsec_list_item_t* last )
parsec_list_item_t* item = first;
do {
assert( item->belong_to == first->belong_to );
- item->refcount--;
+ item->refcount = item->refcount - 1;
assert( 0 == item->refcount );
item = (parsec_list_item_t*)item->list_next;
} while(item != first);
@@ -203,7 +203,7 @@ parsec_list_item_ring_chop( parsec_list_item_t* item )
item->list_prev->list_next = item->list_next;
item->list_next->list_prev = item->list_prev;
#if defined(PARSEC_DEBUG_PARANOID)
- if(item->refcount) item->refcount--;
+ if(item->refcount) item->refcount = item->refcount - 1;
item->list_prev = (parsec_list_item_t*)(void*)0xdeadbeefL;
item->list_next = (parsec_list_item_t*)(void*)0xdeadbeefL;
#endif
@@ -285,7 +285,7 @@ parsec_list_item_ring_push_sorted( parsec_list_item_t* ring,
#define PARSEC_ITEM_ATTACH(LIST, ITEM) \
do { \
parsec_list_item_t *_item_ = (ITEM); \
- _item_->refcount++; \
+ _item_->refcount = _item_->refcount + 1; \
assert( 1 == _item_->refcount ); \
_item_->belong_to = (LIST); \
} while(0)
@@ -309,7 +309,7 @@ parsec_list_item_ring_push_sorted( parsec_list_item_t* ring,
assert( _item->belong_to != (void*)_item ); \
_item->list_prev = (parsec_list_item_t*)(void*)0xdeadbeefL; \
_item->list_next = (parsec_list_item_t*)(void*)0xdeadbeefL; \
- _item->refcount--; \
+ _item->refcount = _item->refcount - 1; \
assert( 0 == _item->refcount ); \
} while (0)
#else |
Do we want to promote this to v4.0 since we have code ready to apply to the problem? |
Describe the bug
Building TTG with GCC 11.4.0 raises a bunch of warnings from PaRSEC:
C++20 has deprecated compound operations on
volatile
variables.A fix is to change
into
Arguably, it's only C++ that deprecated these operations and C apparently has not (yet). However, PaRSEC is now consumed by C++ projects. In the greater scheme of things,
refcount
is probably an atomic variable and should be marked as such (instead ofvolatile
).The text was updated successfully, but these errors were encountered: