[clang] [llvm] [AMDGPU] Add OpenCL-specific fence address space masks (PR #78572)

2024-05-03 Thread Sameer Sahasrabuddhe via cfe-commits 

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)

2024-05-03 Thread Sameer Sahasrabuddhe via cfe-commits 


@@ -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)

2024-05-03 Thread Sameer Sahasrabuddhe via cfe-commits 


@@ -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)

2024-05-03 Thread Sameer Sahasrabuddhe via cfe-commits 


@@ -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)

2024-05-02 Thread Matt Arsenault via cfe-commits 

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)

2024-05-02 Thread Pierre van Houtryve via cfe-commits 

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)

2024-04-26 Thread Matt Arsenault via cfe-commits 


@@ -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)

2024-04-26 Thread Matt Arsenault via cfe-commits 


@@ -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)

2024-04-26 Thread Pierre van Houtryve via cfe-commits 


@@ -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)

2024-04-26 Thread Pierre van Houtryve via cfe-commits 


@@ -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)

2024-04-24 Thread Matt Arsenault via cfe-commits 


@@ -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)

2024-04-24 Thread Matt Arsenault via cfe-commits 


@@ -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)

2024-04-24 Thread Pierre van Houtryve via cfe-commits 

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)

2024-04-24 Thread Pierre van Houtryve via cfe-commits 

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