-
Notifications
You must be signed in to change notification settings - Fork 254
Rebase threads::atomic_int on std::Atomic<int32_t>, avoiding platform specific code #56
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
base: master
Are you sure you want to change the base?
Conversation
| newVal = flagOn ? (prevVal | flag) : (prevVal & ~flag); | ||
|
|
||
| } while(flags.compare_exchange_strong(newVal, prevVal) != prevVal); | ||
| } while( ! flags.compare_exchange_strong(prevVal, newVal)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the std::atomic version of compare_exchange_strong has the arguments switched, and returns bool instead of the value currently being stored.
| void stop() { | ||
| runThread.compare_exchange_strong(1, 2); | ||
| std::int32_t comparand = 2; | ||
| runThread.compare_exchange_strong(comparand, 1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the std::atomic version of compare_exchange_strong has the arguments switched, and returns bool instead of the value currently being stored.
The first argument is also a reference, and has it's value replaced with the value stored in the atomic object at the time the operation is initiated. Previously this would have been the return value.
| if(localFunc) { | ||
| if(runThread.compare_exchange_strong(2, 0) == 0) { | ||
| std::int32_t comparand = 0; | ||
| if(runThread.compare_exchange_strong(comparand, 2)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the std::atomic version of compare_exchange_strong has the arguments switched, and returns bool instead of the value currently being stored.
The first argument is also a reference, and has it's value replaced with the value stored in the atomic object at the time the operation is initiated. Previously this would have been the return value.
| int operator--(int); | ||
|
|
||
| int operator+=(int value); | ||
| int operator-=(int value); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All of these operators are provided by std::atomic<int32_t>
| // Offered for backwards compatibility | ||
| int32_t get_basic() | ||
| { | ||
| return load(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Previously these provided non-atomic get and set.
I didn't see how that would be valid, since it goes against the basic concept of std::atomic. So now they conduct the operations atomically.
Eventually these functions should go away, but they were used in a lot of places and I didn't see any harm in keeping them for now.
|
|
||
| int get() const { return value; } | ||
| operator int() const { return value; } | ||
| void wait_compare_exchange(const int32_t xchg, const int32_t compareToOriginal, const int32_t spinCount) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reimplemented using the std::atomic<>::compare_exchange_strong.
the std::atomic version of compare_exchange_strong has the arguments switched, and returns bool instead of the value currently being stored.
The first argument is also a reference, and has it's value replaced with the value stored in the atomic object at the time the operation is initiated. Previously this would have been the return value.
| int id = getThreadID(); | ||
| if(owningThread != id) { | ||
| for(unsigned i = 0; i < spinCount; ++i) { | ||
| if(owningThread.compare_exchange_strong(id, invalidThreadID) == invalidThreadID) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the std::atomic version of compare_exchange_strong has the arguments switched, and returns bool instead of the value currently being stored.
The first argument is also a reference, and has it's value replaced with the value stored in the atomic object at the time the operation is initiated. Previously this would have been the return value.
| return owningThread == getThreadID(); | ||
| } | ||
|
|
||
| void atomic_int::wait_compare_exchange(int xchg, int compareTo, const int spinCount) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved to the .h file.
|
|
||
| bool Signal::checkAndSignal(int waitFor, int newSignal) { | ||
| return flag.compare_exchange_strong(newSignal, waitFor) == waitFor; | ||
| return flag.compare_exchange_strong(waitFor, newSignal); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the std::atomic version of compare_exchange_strong has the arguments switched, and returns bool instead of the value currently being stored.
The first argument is also a reference, and has it's value replaced with the value stored in the atomic object at the time the operation is initiated. Previously this would have been the return value.
Please note: I did not do extensive testing on this pull request. I'm not actually able to run the game on my development machine. Something's causing the Linux graphics driver to die. Who knows why.
I also did not test that this compiles successfully with any version of Visual Studio. Your mileage may vary.
That being said, I believe this PR should be bug free and very easy for reviewers to go through.