Skip to content
This repository has been archived by the owner on Feb 8, 2024. It is now read-only.

Add support for atomicFetchAdd and atomicFetchSub #193

Merged
merged 1 commit into from
Jan 24, 2021
Merged

Add support for atomicFetchAdd and atomicFetchSub #193

merged 1 commit into from
Jan 24, 2021

Conversation

rymrg
Copy link

@rymrg rymrg commented Jan 19, 2021

@kinke
Copy link
Member

kinke commented Jan 19, 2021

Oh I didn't realize there are public wrappers in core.atomic now (apparently untested...); IIRC, they started out as helper functions in core.internal.atomic for core.atomic.atomicOp(), where LDC doesn't need them, so I left them out back then.

@rymrg
Copy link
Author

rymrg commented Jan 24, 2021

You're right. If I don't use a self built version they don't work. How can I help in adding this for next version? Atomic variables cannot be implemented without this.

/usr/include/dlang/ldc/core/internal/atomic.d(32,28): Error: llvm_atomic_store cannot be interpreted at compile time, because it has no available source code

@kinke
Copy link
Member

kinke commented Jan 24, 2021

How can I help in adding this for next version?

This PR should suffice; please fix the indentation, add an empty line inbetween both new functions, and move them after atomicStore as for the DMD block.

Atomic variables cannot be implemented without this.

atomicOp!"+=" and atomicOp!"-=" don't work? They are tested.

While looking at the upstream code, I've noticed a copy-paste error - atomicFetchSub for pointers using atomicFetchAdd: https://github.com/dlang/druntime/blob/13e358e253e6a420dcfc83244c9479f2db6e8178/src/core/atomic.d#L199
It'd be great if you could fix that upstream too.

@rymrg
Copy link
Author

rymrg commented Jan 24, 2021

Done. I'm not sure how I got the error message from earlier as I cannot reproduce it right now (it was with atomicStore which is required for atomic assignment).
For reference, I also fixed the upstream issue [dlang/pull/3342].

@kinke kinke merged commit 0d1a13f into ldc-developers:ldc Jan 24, 2021
@kinke
Copy link
Member

kinke commented Jan 24, 2021

Thx!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants