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

Support for atomic class variables #18461

Open
bradcray opened this issue Sep 24, 2021 · 7 comments
Open

Support for atomic class variables #18461

bradcray opened this issue Sep 24, 2021 · 7 comments

Comments

@bradcray
Copy link
Member

As a Chapel programmer, I would like support for atomic classes that can be atomically
dereferenced, assigned, swapped, ....

If I recall correctly, one of the traditional high-level challenges here has been that since
Chapel class references are "wide" by default (can point across locales), they essentially
require 128-bit atomics or ... something fancy.

Related issues and PRs:

@mppf
Copy link
Member

mppf commented Sep 27, 2021

Right, we have https://chapel-lang.org/docs/modules/packages/AtomicObjects.html today. There are some meaningful steps we could take to improve this code and make it more production-ready, though. In particular:

  • it only works on x86-64 today
  • it currently only handles up to 65535 locales
  • it uses extern blocks (which IMO is less of an issue today than it used to be, but we could still consider adjusting that as well so that it can work better in a quickstart config)
  • it only works for unmanaged classes but I would imagine that supporting owned as well wouldn't be so hard. (shared might be more challenging due to network latencies around changing the reference count).
  • it needs some API review

@dlongnecke-cray
Copy link
Contributor

Bumping this to say that an atomic c_ptr(...) would also be very much appreciated. I was surprised to see it wasn't supported out of the box. At a glance, I feel like our current atomics framework (especially the lock-free implementations) could readily implement this but I'm not sure how much plumbing is required.

In particular I see atomic_least_uintptr_t is available and we could shove all our pointers into that type under the hood if need be.

@bradcray
Copy link
Member Author

bradcray commented Oct 21, 2024

@dlongnecke-cray : I think it's worth forking that into a separate issue if you're interested. While it feels loosely related to this one, it also feels pretty different.

In the meantime, I wonder whether you could work around this using an approach like this:

use CTypes;

type cPtrAsInt = c_intptr;

var x: atomic cPtrAsInt;
x.add(1);
writeln(x.read());

(along with some casting through c_ptr(void) values)

@dlongnecke-cray
Copy link
Contributor

dlongnecke-cray commented Oct 21, 2024

I actually just went into modules/internal/Atomics.chpl and wired up uintptr_t to map to "local pointer types" (right now just c_ptr) following the framework that was already there. Somebody (was it Elliot?) put a lot of care into making it extremely easy to do 😃.

Michael reminded me that supporting all pointer types will require we make sure we have double word lock free atomic instructions, so there's more of a design discussion to be had there.

Which is exactly what this thread is about! I'll admit that at the time I was just searching for "support atomic for pointer-like things", while forgetting that the main blocker for Chapel is wide pointers 😄.

And I'll back Michael's post about Louis's code - having read and debugged it myself he uses the cas-128 instruction on x86-64. However Michael believes (don't quote me) that there should be pretty standardized support for 128-bit instructions today. So maybe not that hard to do?

@bradcray
Copy link
Member Author

W.r.t. the first bullet in Michael's previous comment, @stonea has since extended the AtomicObjects.chpl package to work on ARM instead.

I actually just went into modules/internal/Atomics.chpl and wired up uintptr_t to map to "local pointer types" (right now just c_ptr) following the framework that was already there.

Thinking about this living on main someday: what are the implications if my code doesn't use CTypes;? And/or is it possible to put the code that wires up uintptr_t / c_ptr into CTypes?

@dlongnecke-cray
Copy link
Contributor

I just did a import CTypes.{c_ptr, c_ptrConst} in Atomics.chpl. I don't know if that's ideal but it was done simply for ease of implementation. It sounds like maybe we would like to do the reverse? What's the aim, reduction in compilation time?

@bradcray
Copy link
Member Author

The main reason I was asking was mild fear that — since Atomics.chpl is automatically compiled into every program that symbols from CTypes would automatically become available to user programs as well. With an import that seems like it shouldn't be the case, but would be good to double-check.

The secondary/lesser reason would be just to reduce the number of modules we compile by default (a general theme I care about, though it hasn't been a focus in recent years) for compilation time reductions as you guessed. And I'd guess that there are many other modules already requiring CTypes to be compiled for every program, so this wouldn't be the worst thing in the world at present (and maybe it's something we'll never realistically get free of and/or shouldn't worry about since we rely on C so much).

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

3 participants