Skip to content

Use proper atomics and ref counting#807

Open
AZero13 wants to merge 1 commit into
swiftlang:mainfrom
AZero13:atomic-3
Open

Use proper atomics and ref counting#807
AZero13 wants to merge 1 commit into
swiftlang:mainfrom
AZero13:atomic-3

Conversation

@AZero13

@AZero13 AZero13 commented Dec 16, 2023

Copy link
Copy Markdown
Contributor

Prefer __atomic_compare_exchange_n over __sync_bool_compare_and_swap.

I chose weak because we are looping and reading the value of old_value constantly anyway, so it would be better to have it weak.

Otherwise, it is equivalent to what it was before.

@ktoso

ktoso commented Dec 18, 2023

Copy link
Copy Markdown
Contributor

@swift-ci please test

@AZero13

AZero13 commented Dec 18, 2023

Copy link
Copy Markdown
Contributor Author

@ktoso My bad! It compiled and ran fine on my machine but I found the mistake and fixed it.

Can we please try one more time?

@AZero13

AZero13 commented Dec 18, 2023

Copy link
Copy Markdown
Contributor Author

@compnerd Can we please do a test?

@compnerd

Copy link
Copy Markdown
Member

I'm not sure that this is valuable enough to do really.

@rjmccall

Copy link
Copy Markdown

I'm not sure what the point of the atomics change is. The control flow change is non-obvious and totally unexplained in either comments or in the PR summary.

@AZero13 AZero13 force-pushed the atomic-3 branch 7 times, most recently from 2da1073 to 5acdd2b Compare December 24, 2023 20:01
Comment thread src/BlocksRuntime/runtime.c Outdated
Prefer __atomic_compare_exchange_n over __sync_bool_compare_and_swap.

I chose weak because we are looping and reading the value of old_value constantly anyway, so it would be better to have it weak.

Otherwise, it is equivalent to what it was before.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants