Skip to content
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

compare_and_swapとtest_and_setで受け取るポインタがvolatileかどうかが統一されていない #84

Open
usatie opened this issue Jan 25, 2023 · 2 comments

Comments

@usatie
Copy link

usatie commented Jan 25, 2023

TASが受け取るboolのポインタpはvolatileなのに、CASが受け取るポインタpはvolatileではなくてもよいのでしょうか?
逆に、test_and_setにおいてvolatileでポインタを受け取る必要があるのでしょうか?

bool	compare_and_swap(uint64_t *p, uint64_t val, uint64_t newval);
bool test_and_set(volatile bool *p);
void tas_release(volatile bool *p);

いくつか関係のありそうな資料やQAを読んでみたのですが、分からなかったので質問形式となってしまうことをご容赦ください。
https://gcc.gnu.org/onlinedocs/gcc-4.1.0/gcc/Atomic-Builtins.html
https://stackoverflow.com/questions/6023310/sync-bool-compare-and-swap-compiler-flags-and-includes
https://stackoverflow.com/questions/4557979/when-to-use-volatile-with-multi-threading

@ytakano
Copy link
Collaborator

ytakano commented Jan 29, 2023

ご指摘は、全くその通りです。
ここは、volatileをつけるべきに思われます。

では、なぜvolatileをつけなかったかというと、執筆時にテストしたところ、clangかgccか忘れましたがvolatileをつけるとコンパイラがwarningを出力するからで、どちらにすべきか迷った結果、volatileを省略しています。

volatileをつけた方が良かったのかとも思っています。

@usatie
Copy link
Author

usatie commented Jan 29, 2023

お返事ありがとうございます。

volatileはコンパイラの最適化を防ぐためということだったので、ループをしていない今回のようなケースではつけるそもそも必要なかったりするのでしょうか。

ちなみに、以下のファイルを私の環境(m1 macbookair)でコンパイルしてみてもvolatileがついていることによるエラーは出ませんでした。
gcc -c 3-2.c

#include <stdbool.h>
#include <stdint.h>

bool	compare_and_swap(volatile uint64_t *p, uint64_t val, uint64_t newval);
bool	test_and_set(volatile bool *p);
void	tas_release(volatile bool *p);

// Why p is non volatile?
bool	compare_and_swap(volatile uint64_t *p, uint64_t val, uint64_t newval)
{
	return __sync_bool_compare_and_swap(p, val, newval);
}

bool	test_and_set(volatile bool *p)
{
	return __sync_lock_test_and_set(p, 1);
}

void	tas_release(volatile bool *p)
{
	return __sync_lock_release(p);
}

gcc -v

Apple clang version 14.0.0 (clang-1400.0.29.202)
Target: arm64-apple-darwin22.2.0
Thread model: posix
InstalledDir: /Library/Developer/CommandLineTools/usr/bin

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

No branches or pull requests

2 participants