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

runtime,sync/atomic: unify API for runtime/internal/atomic and sync/atomic #8739

Open
dvyukov opened this issue Sep 15, 2014 · 12 comments
Open
Milestone

Comments

@dvyukov
Copy link
Member

dvyukov commented Sep 15, 2014

We need to unify interface of atomic operations between runtime package and sync/atomic
package. And then teach gc to replace functions in both packages with intrinsics where
possible.

This is followup to https://golang.org/issue/4947 which was about
atomic intrinsics for runtime C code.
@rsc
Copy link
Contributor

rsc commented May 19, 2015

There's no evidence in profiles that this is important for Go 1.5.

@rsc rsc modified the milestones: Unplanned, Go1.5 May 19, 2015
@rsc rsc changed the title cmd/gc: atomic intrinsics for runtime and sync/atomic cmd/compile: atomic intrinsics for runtime and sync/atomic Jun 8, 2015
@dsnet
Copy link
Member

dsnet commented Jan 4, 2017

\cc @randall77, I vaguely remember you mentioning that we already intrinsify atomic.X functions. Is this done?

@randall77
Copy link
Contributor

They are for 1.8. We intrinsify both sync/atomic and runtime/internal/atomic for a bunch of architectures.
The APIs are not unified (e.g. LoadUint32 in sync/atomic is Load in runtime/internal/atomic).
We should probably leave this bug open for that unification. It shouldn't be hard.

@laboger
Copy link
Contributor

laboger commented Feb 2, 2017

I'm planning to implement the atomics as intrinsics on ppc64x. Should I refer to this issue in my CL or open a new one.

@randall77
Copy link
Contributor

You can reference this issue.

@laboger
Copy link
Contributor

laboger commented Feb 10, 2017

@randall77, I've had a change to intrinsify atomics for ppc64x that worked fine a few months ago. When I tried to rebase it, now it hangs during the build. I've tracked it down to commit 8a9dc05 to allow inlining of functions with intrinsics in them. If I remove the code from that commit, then my change to intrinsify atomics works again.

@randall77
Copy link
Contributor

@laboger Can you point me at your intrinsics CL?

@laboger
Copy link
Contributor

laboger commented Feb 10, 2017

@randall77 I didn't create one yet since it didn't build. But I will do that now.

@gopherbot
Copy link

CL https://golang.org/cl/36832 mentions this issue.

@laboger
Copy link
Contributor

laboger commented Feb 10, 2017

CL #36832

@mark-rushakoff
Copy link
Contributor

Was this issue supposed to be closed with CL 36832? The CL is specific to PPC64x but it looks like this issue is not specific to any architecture.

@randall77
Copy link
Contributor

No, we should leave it open. I think the remaining task is unifying the apis for sync/atomic and runtime/internal/atomic.

@randall77 randall77 reopened this Mar 2, 2017
@randall77 randall77 changed the title cmd/compile: atomic intrinsics for runtime and sync/atomic runtime,sync/atomic: unify API for runtime/internal/atomic and sync/atomic Apr 2, 2019
@rsc rsc removed their assignment Jun 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants