[clang] [llvm] [AMDGPU] Add OpenCL-specific fence address space masks (PR #78572)
ssahasra wrote: > Should we also rename the MMRA to `amdgpu-fence-as` (remove OpenCL from the > name) ? Even the "fence" prefix is not entirely correct. The same tags also make sense on a load-acquire or store-release, which are "fence like" instructions, or "operations with implicit fences". Why not just "as:global", "as:local", etc? The fact that they are used as !mmra on a fence-like instruction makes it clear that they represent the address spaces that are caused to be synchronized by that instruction, and not incidentally the address space that the load-acquire or store-release itself wants to access. https://github.com/llvm/llvm-project/pull/78572 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [AMDGPU] Add OpenCL-specific fence address space masks (PR #78572)
@@ -4408,6 +4409,54 @@ Target-Specific Extensions Clang supports some language features conditionally on some targets. +AMDGPU Language Extensions +-- + +__builtin_amdgcn_fence +^^ + +``__builtin_amdgcn_fence`` emits a fence for all address spaces +and takes the following arguments: + +* ``unsigned`` atomic ordering, e.g. ``__ATOMIC_ACQUIRE`` +* ``const char *`` synchronization scope, e.g. ``workgroup`` + +.. code-block:: c++ + + __builtin_amdgcn_fence(__ATOMIC_SEQ_CST, "workgroup"); + __builtin_amdgcn_fence(__ATOMIC_ACQUIRE, "agent"); + +__builtin_amdgcn_masked_fence +^ + +``__builtin_amdgcn_masked_fence`` emits a fence for one or more address +spaces and takes the following arguments: + +* ``unsigned`` atomic ordering, e.g. ``__ATOMIC_ACQUIRE`` +* ``const char *`` synchronization scope, e.g. ``workgroup`` +* Zero or more ``const char *`` address spaces. + +The address spaces arguments must be string literals with known values, such as: + +* ``"local"`` +* ``"global"`` +* ``"image"`` + +If there are no address spaces specified, this fence behaves like ssahasra wrote: To answer comments elsewhere, this documentation makes it really obvious that the two fences are really just one. https://github.com/llvm/llvm-project/pull/78572 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [AMDGPU] Add OpenCL-specific fence address space masks (PR #78572)
@@ -18365,6 +18366,30 @@ Value *CodeGenFunction::EmitHLSLBuiltinExpr(unsigned BuiltinID, return nullptr; } +void CodeGenFunction::AddAMDGCNAddressSpaceMMRA(llvm::Instruction *Inst, +const CallExpr *E, +unsigned FirstASNameIdx) { + constexpr const char *Tag = "opencl-fence-mem"; ssahasra wrote: Defintely drop the "opencl" prefix. https://github.com/llvm/llvm-project/pull/78572 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [AMDGPU] Add OpenCL-specific fence address space masks (PR #78572)
@@ -18365,6 +18366,30 @@ Value *CodeGenFunction::EmitHLSLBuiltinExpr(unsigned BuiltinID, return nullptr; } +void CodeGenFunction::AddAMDGCNAddressSpaceMMRA(llvm::Instruction *Inst, +const CallExpr *E, +unsigned FirstASNameIdx) { + constexpr const char *Tag = "opencl-fence-mem"; + + LLVMContext = Inst->getContext(); + SmallVector MMRAs; + for (unsigned K = FirstASNameIdx; K < E->getNumArgs(); ++K) { +llvm::Value *V = EmitScalarExpr(E->getArg(K)); +StringRef AS; +if (llvm::getConstantStringInfo(V, AS) && +(AS == "local" || AS == "global" || AS == "image")) { ssahasra wrote: Can these magic strings be declared somewhere? Ideally some sort of target-specific mechanism that Clang will use to find such sets of valid strings? https://github.com/llvm/llvm-project/pull/78572 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [AMDGPU] Add OpenCL-specific fence address space masks (PR #78572)
arsenm wrote: > I'm now wondering if adding a new builtin is needed at all, or if it should > just be part of the original builtin? It's an additive change. Maybe? > > Should we also rename the MMRA to `amdgpu-fence-as` (remove OpenCL from the > name) ? > I definitely do not want to maintain any language names in anything https://github.com/llvm/llvm-project/pull/78572 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [AMDGPU] Add OpenCL-specific fence address space masks (PR #78572)
Pierre-vh wrote: I changed it so it's one or more string arguments: ``` __builtin_amdgcn_masked_fence(__ATOMIC_SEQ_CST, "workgroup", "local", "global") ``` I'm now wondering if adding a new builtin is needed at all, or if it should just be part of the original builtin? It's an additive change. Should we also rename the MMRA to `amdgpu-fence-as` (remove OpenCL from the name) ? Then this feature would become fully generic. https://github.com/llvm/llvm-project/pull/78572 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [AMDGPU] Add OpenCL-specific fence address space masks (PR #78572)
@@ -18319,6 +18320,26 @@ Value *CodeGenFunction::EmitHLSLBuiltinExpr(unsigned BuiltinID, return nullptr; } +void CodeGenFunction::AddAMDGCNAddressSpaceMMRA(llvm::Instruction *Inst, +llvm::Value *ASMask) { + constexpr const char *Tag = "opencl-fence-mem"; + + uint64_t Mask = cast(ASMask)->getZExtValue(); + if (Mask == 0) +return; + + // 3 bits can be set: local, global, image in that order. + LLVMContext = Inst->getContext(); + SmallVector MMRAs; + if (Mask & (1 << 0)) arsenm wrote: Space separated is weird. I meant more __builtin_amdgcn_something_fence("somesyncscope", ordering, "addrspace0", "addrspace1", ...) https://github.com/llvm/llvm-project/pull/78572 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [AMDGPU] Add OpenCL-specific fence address space masks (PR #78572)
@@ -4403,6 +4404,60 @@ Target-Specific Extensions Clang supports some language features conditionally on some targets. +AMDGPU Language Extensions +-- + +__builtin_amdgcn_fence +^^ + +``__builtin_amdgcn_fence`` emits a fence for all address spaces +and takes the following arguments: + +* ``unsigned`` atomic ordering, e.g. ``__ATOMIC_ACQUIRE`` +* ``const char *`` synchronization scope, e.g. ``workgroup`` + +.. code-block:: c++ + + __builtin_amdgcn_fence(__ATOMIC_SEQ_CST, "workgroup"); + __builtin_amdgcn_fence(__ATOMIC_ACQUIRE, "agent"); + +__builtin_amdgcn_masked_fence +^ + +``__builtin_amdgcn_masked_fence`` emits a fence for one or more address +spaces. + +* ``unsigned`` address space mask +* ``unsigned`` atomic ordering, e.g. ``__ATOMIC_ACQUIRE`` +* ``const char *`` synchronization scope, e.g. ``workgroup`` + +The address space mask is a bitmask and each bit corresponds +to an OpenCL memory type: + +* ``0x1`` (first bit) is for local memory. +* ``0x2`` (second bit) is for global memory. +* ``0x4`` (third bit) is for image memory. arsenm wrote: Should either get named enums or string-like treatment like the sync scope https://github.com/llvm/llvm-project/pull/78572 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [AMDGPU] Add OpenCL-specific fence address space masks (PR #78572)
@@ -69,6 +69,7 @@ BUILTIN(__builtin_amdgcn_iglp_opt, "vIi", "n") BUILTIN(__builtin_amdgcn_s_dcache_inv, "v", "n") BUILTIN(__builtin_amdgcn_buffer_wbinvl1, "v", "n") BUILTIN(__builtin_amdgcn_fence, "vUicC*", "n") +BUILTIN(__builtin_amdgcn_masked_fence, "vUiUicC*", "n") Pierre-vh wrote: Added docs with tests. I also documented the other fence intrinsic https://github.com/llvm/llvm-project/pull/78572 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [AMDGPU] Add OpenCL-specific fence address space masks (PR #78572)
@@ -18319,6 +18320,26 @@ Value *CodeGenFunction::EmitHLSLBuiltinExpr(unsigned BuiltinID, return nullptr; } +void CodeGenFunction::AddAMDGCNAddressSpaceMMRA(llvm::Instruction *Inst, +llvm::Value *ASMask) { + constexpr const char *Tag = "opencl-fence-mem"; + + uint64_t Mask = cast(ASMask)->getZExtValue(); + if (Mask == 0) +return; + + // 3 bits can be set: local, global, image in that order. + LLVMContext = Inst->getContext(); + SmallVector MMRAs; + if (Mask & (1 << 0)) Pierre-vh wrote: @arsenm Do you mean just having a space-separate list of address spaces such as `"image local"`? If that's acceptable then it definitely works for me too https://github.com/llvm/llvm-project/pull/78572 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [AMDGPU] Add OpenCL-specific fence address space masks (PR #78572)
@@ -69,6 +69,7 @@ BUILTIN(__builtin_amdgcn_iglp_opt, "vIi", "n") BUILTIN(__builtin_amdgcn_s_dcache_inv, "v", "n") BUILTIN(__builtin_amdgcn_buffer_wbinvl1, "v", "n") BUILTIN(__builtin_amdgcn_fence, "vUicC*", "n") +BUILTIN(__builtin_amdgcn_masked_fence, "vUiUicC*", "n") arsenm wrote: Should put some documentation in LanguageExtensions for the arguments https://github.com/llvm/llvm-project/pull/78572 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [AMDGPU] Add OpenCL-specific fence address space masks (PR #78572)
@@ -18319,6 +18320,26 @@ Value *CodeGenFunction::EmitHLSLBuiltinExpr(unsigned BuiltinID, return nullptr; } +void CodeGenFunction::AddAMDGCNAddressSpaceMMRA(llvm::Instruction *Inst, +llvm::Value *ASMask) { + constexpr const char *Tag = "opencl-fence-mem"; + + uint64_t Mask = cast(ASMask)->getZExtValue(); + if (Mask == 0) +return; + + // 3 bits can be set: local, global, image in that order. + LLVMContext = Inst->getContext(); + SmallVector MMRAs; + if (Mask & (1 << 0)) arsenm wrote: Alternatively could behave more like system scopes where the builtin takes a variadic list of string literal arguments passed directly through https://github.com/llvm/llvm-project/pull/78572 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [AMDGPU] Add OpenCL-specific fence address space masks (PR #78572)
https://github.com/Pierre-vh edited https://github.com/llvm/llvm-project/pull/78572 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [AMDGPU] Add OpenCL-specific fence address space masks (PR #78572)
https://github.com/Pierre-vh edited https://github.com/llvm/llvm-project/pull/78572 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits