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

cmd/compile: atomic intrinsics for runtime and sync/atomic #8739

Open
dvyukov opened this Issue Sep 15, 2014 · 12 comments

Comments

Projects
None yet
8 participants
@dvyukov
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.

@bradfitz bradfitz modified the milestone: Go1.5 Dec 16, 2014

@bradfitz bradfitz removed the release-go1.5 label Dec 16, 2014

@rsc rsc removed accepted labels Apr 14, 2015

@rsc

This comment has been minimized.

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 from cmd/gc: atomic intrinsics for runtime and sync/atomic to cmd/compile: atomic intrinsics for runtime and sync/atomic Jun 8, 2015

@dsnet

This comment has been minimized.

Member

dsnet commented Jan 4, 2017

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

@randall77

This comment has been minimized.

Contributor

randall77 commented Jan 4, 2017

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

This comment has been minimized.

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

This comment has been minimized.

Contributor

randall77 commented Feb 2, 2017

You can reference this issue.

@laboger

This comment has been minimized.

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

This comment has been minimized.

Contributor

randall77 commented Feb 10, 2017

@laboger Can you point me at your intrinsics CL?

@laboger

This comment has been minimized.

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

This comment has been minimized.

gopherbot commented Feb 10, 2017

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

@laboger

This comment has been minimized.

Contributor

laboger commented Feb 10, 2017

CL #36832

@gopherbot gopherbot closed this in 95c9583 Mar 1, 2017

@mark-rushakoff

This comment has been minimized.

Contributor

mark-rushakoff commented Mar 2, 2017

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

This comment has been minimized.

Contributor

randall77 commented Mar 2, 2017

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment