-
Notifications
You must be signed in to change notification settings - Fork 1.6k
fix define for YIELDING #5579
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: develop
Are you sure you want to change the base?
fix define for YIELDING #5579
Conversation
|
Alternatively if the intent wasn't to override this, then I can swap the order of the declarations to define the architecture first. |
|
intention is to override, but I haven't checked which of the two is actually preferable on Windows - though I suspect it will be the asm no-op that currently "wins" amid the noisy warning. |
|
Alright, can change that. I see past issues now that indicate why it uses the spin lock with nops (#1881 (comment)) |
|
seems to be missing an |
The intent is to define this as nop, but previously it was then immediately overriding it for various architectures, causing a compiler warning on Windows.
|
ah, yep, it was missing at the obvious place |
| #define YIELDING SwitchToThread() | ||
|
|
||
| #else | ||
| #else // assume linux |
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.
not only linux but *bsd as well - and probably other unixoid systems as well - this is just "assume sched_yield may work better with an unknown scheduler than doing nothing at all", and was historically what GotoBLAS/OpenBLAS would use on any host. perhaps simply leave out the comment ?
The intent seem to be to define this in one way, but previously it was
then immediately overriding it for various architectures, causing a
compiler warning.