Note that outside the GPU world, "shared memory" typically refers to memory shared between processes. So I would suggest using a name that's less likely to be confused, like something that explicitly involves "GPU" or so.

This sounds like a form of "global" memory (similar to a static item), but then apparently OpenCL calls it "local" which is very confusing...

Does it make sense to add a mod gpu?
I think there are more intrinsics for gpus that make can be added (although more in the traditional intrinsic sense, relating to an instruction, edit: re-exposing intrinsics from core::arch::nvptx and the amdgpu equivalent).

Or should it be in core::arch::gpu?
(From #135516 (comment), cc @workingjubilee)

Rust intrinsic names are not namespaced. They are exposed in a module, but inside the compiler they are identified entirely by their name. So moving them into a different module doesn't alleviate the need for a clear name that will be understandable to non-GPU people working in the compiler (which is the vast majority of compiler devs).

If there's more GPU intrinsics to come, moving them into a gpu.rs file here still might make sense.

I don't have a strong opinion on how the eventually stable public API is organized, I am commenting entirely as someone who has an interest in keeping the set of intrinsics the Rust compiler offers understandable and well-defined (the ones in this folder, not the ones in core::arch which you call "more traditional" but that's very dependent on your background ;). These intrinsics are just an implementation detail, but every intrinsic we add here is a new language primitive -- it's like adding a new keyword, just without the syntax discussions and perma-unstable. In the past we used to have intrinsics that entirely break the internal consistency of the language, and we used to have intrinsics whose safety requirements were very poorly documented.

Does this mean that all dynamic_shared_memory::<T> for the same T return the same pointer, or do even dynamic_shared_memory::<T> and dynamic_shared_memory::<U> point to the same memory?

All of them alias, independent of the type.
It’s probably worth explaining the concept of shared memory in the comment?

Maybe this makes it clearer:

Returns a pointer to dynamic shared memory.

Shared memory is a memory region that is shared between all threads in
the same block/work-group. It is usually faster than global memory, which is
shared between all threads on a GPU.
Dynamic shared memory is in the shared memory region, though the allocated
size is specified late, when launching a gpu-kernel.

The pointer returned by dynamic_shared_memory() is the start of the dynamic
shared memory region. All calls to dynamic_shared_memory in a block/work-group,
independent of the generic type, return the same address, so alias the same memory.
The returned pointer is aligned by at least the alignment of T.

Other APIs

CUDA and HIP call this shared memory, shared between threads in a block.
OpenCL and SYCL call this local memory, shared between threads in a work-group.
GLSL calls this shared memory, shared between invocations in a work group.
DirectX calls this groupshared memory, shared between threads in a thread-group.

It’s probably worth explaining the concept of shared memory in the comment?

It's probably worth using a more specific term ("GPU shared memory" or so) since many people reading this will think "shared memory" refers to its more standard meaning of memory shared across different processes (often set up via mmap). It's unfortunate that GPU vendors chose to overload this term, but when working in a more general-purpose codebase you can't just assume everyone to know the conventions of the GPU community, and you can't give general-purpose terms a GPU-specific meaning without risking confusion.

Other APIs

CUDA and HIP call this shared memory, shared between threads in a block.
OpenCL and SYCL call this local memory, shared between threads in a work-group.
GLSL calls this shared memory, shared between invocations in a work group.
DirectX calls this groupshared memory, shared between threads in a thread-group.

This sort of "translation guide" is not actually useful if you are not familiar with any of these things, so I would just leave it out as it is a distraction from the actual description. Especially since it's very easy to go look up a definition of, say, OpenCL's local memory, see it referred to as "this is GLSL's shared memory", look up a definition of that and see it referred to as basically the same idea as groupshared memory in DirectX, then look up a definition of that and... you get the idea. Our definition should stand on its own.

The translation guide might be useful to people that are familiar with these things and wondering why we are making up our own terms.

What does this do when there are multiple translation units? It seems like each would declare its own version of the global, potentially with a different alignment?

All of them return the same pointer.
I assume the linker ensures the maximum alignment is taken and the runtime will ensure that it’s honored.
(Though note that multiple translation units are currently not really supported in the backends, they both rely on IR linking.)

All of them return the same pointer.

By which magic does that happen? That's not usually how LLVM globals work, AFAIK.

I assume the linker ensures the maximum alignment is taken and the runtime will ensure that it’s honored.

That would be rather surprising to me; usually a global can only be declared in one translation unit and then must be imported in all the others. Your current implementation might end up with a separate symbol for each TU, I am not sure.

EDIT: Ah I didn't realize these are extern. The name LLVMRustGetOrInsertGlobal does not make that clear. I understand the name is pre-existing.

(Though note that multiple translation units are currently not really supported in the backends, they both rely on IR linking.)

How does that work in rustc where each crate is a separate TU?

Yes, the key is that they are extern. I agree it would not make sense for non-extern globals.

How does that work in rustc where each crate is a separate TU?

We do (fat/full) LTO. All crates compile to bitcode, than that bitcode is linked and compiled as one LLVM IR module.
The Rust amdgpu backend uses the existing lto/linker-plugin-lto infrastructure, the Rust nvptx backend is declaring IR/bitcode as the output format and building the bitcode-linker, to link them together.

Usually an alignment attribute on an extern global encodes an assumption -- it's a promise to LLVM that someone else has aligned the global sufficiently.

It looks like this here relies on some sort of magic handshake where whatever component actually creates the global will honor whatever alignment you can find in the bitcode?

That's pretty fragile -- usually in LLVM it's always correct to reduce the alignment attribute on such declarations, it just means we know less about what the real alignment is. (This is in contrast to alignment annotations on definitions which defines the real alignment, so it can be safely increased but not reduced.) Seems like some system here is (ab)using LLVM attributes in ways that break some of their core properties, or am I misunderstanding something?

Is this magic handshake documented anywhere?

Is there a special magic meaning to this hard-coded name for the global?

When linking together Rust code and code in other languages, is this the name everyone must use consistently to ensure things behave correctly?

The name has no meaning.
I didn’t try, but I’m rather confident to claim that all external addrspace(3) globals alias. There is no way to get two different dynamic shared memory pointers.
Using the name here and updating the global’s alignment is to generate readable and “clean” LLVM IR.

I’m rather confident to claim that all external addrspace(3) globals alias

That seems very unusual, so if the intrinsic impl relies on this there should be a comment referring to the place where this is documented.

This should have a name that makes it more clear that it is GPU-specific.

Speaking of safety requirements... how does one use this pointer? I get that it is aligned, but does it point to enough memory to store a T? If it's always the same address, doesn't everyone overwrite each other's data all the time? This API looks very odd for a non-GPU person, and it's not clear to me whether that is resolved by having more magic behavior (which should be documented or at least referenced here), or whether there's higher-level APIs built on top that deal with this (but this intrinsic provides so few guarantees, I can't see how that should be possible).

Typically, intrinsic documentations should be detailed enough that I can read and write code using the intrinsic and know exactly whether the code is correct and what it will do in all circumstances. I don't know if there's any hope of achieving that with GPU intrinsics, but if not then we need to have a bit of a wider discussion -- we have had bad experience with just importing "externally defined" semantics into Rust without considering all the interactions (in general, it is not logically coherent to have semantics externally defined).

The current docs would let me implement this intrinsic by just always returning 1024, and emitting a compile error if T has alignment bigger than 1024. I doubt that's a legal implementation. But that means the docs are not precise enough to describe what the implementation must do.

The ?Sized here is almost certainly wrong. The implementation uses align_of which only makes sense for sized types. Unsized types don't have an alignment by themselves, you need to know the wide ptr metadata to determine the alignment.

Is there some prior discussion of the design decision to determine the alignment by giving a type parameter? I could also be a const generic parameter, for instance. I don't have an opinion on the matter since I am an outsider to the GPU world, but as a compiler team member it'd be good to know if this is something you thought about for 5 minutes or whether there's some sort of larger design by a team that has a vision of how all these things will fit together.

There is some discussion in #135516. I don’t mind either way, I thought (for 5 minutes ;)) that specifying the type of the returned pointer makes sense.
I’m not much of a GPU programmer, but I think in most cases, you would store an array in dynamic shared memory, or maybe a struct followed by a dynamically sized array (or maybe two/n arrays of different types).

For just a struct, static shared memory would make more sense, though we don’t support that yet (there’s some discussion in the tracking issue, but I think that’s more complicated to design and implement).

Would using mini-core instead of the precompiled core in the sysroot work instead of hard-coding -Ctarget-cpu=gfx900 in the build system? That way you can also omit the only-amdgpu and only-nvptx64 directives and instead make it run whenever LLVM supports it.

I don't think hardcoding gfx900 as the target will work at all, different uarches can have wildly different codegen'd binaries and are usually not backwards compatible. I think this is a case where we want the user to specify the target architecture that they are compiling for.

This would probably work for smoke testing but in the long term it's probably going to cause more pains than it's worth.

Given that this is just about local testing that the compiler works at all, none of the binary compatibility concerns really matter to us. This code is not executed.

But I agree that we should probably just use the minicore for this instead of adding code to the build system that we might have to rip out later. It's easy to update tests... unless they are built on a fragile assumption in the build system.

I could probably duplicate the intrinsic in minicore, but wouldn’t that defeat the purpose of this test?
The goal here is to test the intrinsic that is added to core.

I think this is a case where we want the user to specify the target architecture that they are compiling for.

Sure, that’s what the target enforces :) Unfortunately it’s the exact thing that throws a wrench into writing tests for the standard library.

Duplicating the intrinsic declaration is fine; what you are testing is the implementation and that is not being duplicated.

This is not enough when running on x86_64 Windows. You did need only-x86_64-unknown-linux-gnu I think.

"shared" between whomst?

The concept is called “shared memory”, so I meant it as a reference to that concept, not as an explanation ;)
I’ll make this more descriptive (though as at stands, we’ll first need to decide on nomenclature to use throughout the Rust codebase).

I know this is a short description, but it is just repeating the function signature:

pub fn dynamic_shared_memory<T: ?Sized>() -> *mut T;
/*     ^^^^^^^^^^^^^^^^^^^^               ^^ ^^^^^^
       |                                  |  |
       |                                  |  |
       -- "dynamic shared memory"         |  - "a pointer"
                                          - "returns"
*/

I'm basically repeating the same comment I already made about "shared with what?" except with adding that "dynamic" also is pretty vague since in computer programming it's always relative to some notion of "static". Yet the "static" is not obviously present in this description.

Given that this is just about local testing that the compiler works at all, none of the binary compatibility concerns really matter to us. This code is not executed.

But I agree that we should probably just use the minicore for this instead of adding code to the build system that we might have to rip out later. It's easy to update tests... unless they are built on a fragile assumption in the build system.

Other APIs

CUDA and HIP call this shared memory, shared between threads in a block.
OpenCL and SYCL call this local memory, shared between threads in a work-group.
GLSL calls this shared memory, shared between invocations in a work group.
DirectX calls this groupshared memory, shared between threads in a thread-group.

This sort of "translation guide" is not actually useful if you are not familiar with any of these things, so I would just leave it out as it is a distraction from the actual description. Especially since it's very easy to go look up a definition of, say, OpenCL's local memory, see it referred to as "this is GLSL's shared memory", look up a definition of that and see it referred to as basically the same idea as groupshared memory in DirectX, then look up a definition of that and... you get the idea. Our definition should stand on its own.

Hm. I'm not sure this is the correct design for this, because it makes the calls to an intrinsic alter global program state during compilation, which feels very dicey and assumes that the calls to that intrinsic will experience codegen. I'm not sure how substantial this concern is, however, so I'm willing to be persuaded.

Ah good point... if you call this function in dead code, it may or may not have any effect.

How do other languages handle controlling the alignment of this magic global?

To be fair, even if it is called, it does not have any effect. It’s just a getter for a pointer that exists anyway. It does not change anything if it’s dead-code-eliminated.
So, to make it clear, calling the intrinsic does not alter any program state.

We're talking about the funky behavior where DCE'ing the intrinsic removes its compile-time effect on the alignment.