[libclc] Support the generic address space

[frasercrmck]

This commit provides definitions of builtins with the generic address space.

One concept to consider is the difference between supporting the generic address space from the user's perspective and the requirement for libclc as a compiler implementation detail to define separate generic address space builtins. In practice a target (like NVPTX) might notionally support the generic address space, but it's mapped to the same LLVM target address space as another address space (often the private one).

In such cases libclc must be careful not to define both private and generic overloads of the same builtin. We track these two concepts separately, and make the assumption that if the generic address space does clash with another, it's with the private one. We track the concepts separately because there are some builtins such as atomics that are defined for the generic address space but not the private address space.

[frasercrmck]

CC @wenju-he

[arsenm]

Even as a compiler implementation detail, libclc should not need to consider the address space mapping (unless maybe you're directly using IR)

There is a clang bug if there is different mangling. The itanium mangling should be coming from the source type / original address space, not whatever IR address space value that happens to map to

[frasercrmck]

There is a clang bug if there is different mangling. The itanium mangling should be coming from the source type / original address space, not whatever IR address space value that happens to map to

Yeah, that would be nice but this is what's happening, I'm afraid.

It is actually supported in the Itanium mangler:

    if (Context.getASTContext().addressSpaceMapManglingFor(AS)) {
      //  <target-addrspace> ::= "AS" <address-space-number>
      unsigned TargetAS = Context.getASTContext().getTargetAddressSpace(AS);
      if (TargetAS != 0 ||
          Context.getASTContext().getTargetAddressSpace(LangAS::Default) != 0)
        ASString = "AS" + llvm::utostr(TargetAS);
    } else {
      switch (AS) {
      default: llvm_unreachable("Not a language specific address space");
      //  <OpenCL-addrspace> ::= "CL" [ "global" | "local" | "constant" |
      //                                "private"| "generic" | "device" |
      //                                "host" ]
      case LangAS::opencl_global:
        ASString = "CLglobal";
        break;

It's just that targets we care about in libclc unconditionally enable that address space map mangling for all address spaces, such as AMDGPU and NVPTX.

I'm not sure I would want to change this behaviour at this point. At least not for the purposes of enabling generic address space support in libclc. There will be a bunch of downstream toolchains that rely on the current mangling scheme.

[arsenm]

It is actually supported in the Itanium mangler:

I don't remember this part of the hack. There was a recent fix to always use the correct mapping values for AMDGPU when generic address space is enabled (which should be the only mapping, still need to do something about the setAddressSpaceMap hack).

[arsenm]

Yes, the extension should be reported as available or not by the target macros

[wenju-he]

we should enable __opencl_c_generic_address_space for amdgpu and nvptx in setSupportedOpenCLOpts API rather than in this CMakeLists file, right?

[arsenm]

yes (although for amdgpu it needs to skip the ancient targets without flat addressing)

[frasercrmck]

See the first PR for AMDGPU support: #137636.

I'll do a separate one for NVPTX. It looks like SPIRV (and X86) enable all by default. That should cover all libclc targets.

[frasercrmck]

With #137940 we can remove this CMake logic, thanks all!

One result of #137636 is that (I believe) because of the default AMDGPU devices we compile libclc for, the generic address space isn't being enabled by default for any AMDGPU target in this PR. Should we perhaps be building libclc for a newer AMDGCN device? It should be at least GFX700 for generic address space support.

[arsenm]

There isn't a generic target you can just compile for. For amdhsa triples, we assume the default dummy target-cpu supports flat pointers. -mlink-builtin-bitcode plus a collection of other hacks let's us get pretty far in pretending we can have a generic bitcode, but it's fragile system I would like to get away from.

Long term I think we need better compartmentalization on target feature dependence. i.e. we should have a base implementation plus the compiler can select target specific function variants later. In this case the generic address space is pretty fundamental. I doubt anyone is regularly testing with gfx6 anywhere, maybe we could just drop them from the support list here. It also shouldn't be that part to implement software tagged flat pointers, there would just be a small runtime component required to make it work (which is driver work which would never really be implemented)

[frasercrmck]

That's probably a good idea, yes. There's a lot of redundancy in having these large bytecode libraries which are 95% functionally equivalent between targets/architectures.

I can circle back to this in a follow-up PR to investigate the enabling of the generic address space support for AMDGPU architectures.

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.