Skip to content

Conversation

wenju-he
Copy link
Contributor

@wenju-he wenju-he commented Jul 31, 2025

__clc_mem_fence and __clc_work_group_barrier function have two
parameters memory_scope and memory_order. The design allows the clc
functions to implement SPIR-V ControlBarrier and MemoryBarrier
functions in the future.

The default memory ordering in clc is set to __ATOMIC_SEQ_CST, which is
also the default and strongest ordering in OpenCL and C++.

OpenCL cl_mem_fence_flags parameter is converted to combination of
__MEMORY_SCOPE_DEVICE and __MEMORY_SCOPE_WRKGRP, which is passed to clc.

llvm-diff shows no change to nvptx64--nvidiacl.bc.
llvm-diff show a small change to amdgcn--amdhsa.bc and the number of
LLVM IR instruction is reduced by 1: https://alive2.llvm.org/ce/z/_Uhqvt

__clc_mem_fence and __clc_barrier function have two parameters Scope and
MemorySemantics, which are defined in SPIR-V spec. The design allows the
clc functions to implement SPIR-V ControlBarrier and MemoryBarrier
functions in the future.

The default memory ordering in clc is set to SequentiallyConsistent,
which is also the default and strongest ordering in OpenCL and C++.

The default memory scope in clc is set to memory_scope_device for amdgcn
and ptx-nvidiacl since __opencl_c_atomic_scope_all_devices feature macro
is not defined for these targets.

llvm-diff shows no change to amdgcn--amdhsa.bc and nvptx64--nvidiacl.bc.
@llvmbot llvmbot added the libclc libclc OpenCL library label Jul 31, 2025
@wenju-he wenju-he requested review from frasercrmck and arsenm July 31, 2025 04:13
#define __CLC_MEM_FENCE_CLC_MEM_SCOPE_SEMANTICS_H__

// Scope values are defined in SPIR-V spec.
typedef enum Scope {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should stick to OpenCL spec and terminology; I thought clang already had predefined enums for both of these

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should stick to OpenCL spec and terminology; I thought clang already had predefined enums for both of these

done, thanks @arsenm
Changed to use clang macro __MEMORY_SCOPE_DEVICE/__MEMORY_SCOPE_WRKGRP and __ATOMIC_SEQ_CST

@wenju-he wenju-he requested a review from arsenm August 5, 2025 11:04

#include <clc/mem_fence/clc_mem_fence.h>

void __clc_amdgcn_s_waitcnt(unsigned flags);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should get rid of this; it can be properly implemented with __builtin_amdgcn_fence

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should get rid of this; it can be properly implemented with __builtin_amdgcn_fence

I'll follow up

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should get rid of this; it can be properly implemented with __builtin_amdgcn_fence

I'll follow up

re-implement with __builtin_amdgcn_fence: #152275

@wenju-he wenju-he merged commit af16fc2 into llvm:main Aug 6, 2025
9 checks passed
@wenju-he wenju-he deleted the libclc-barrier branch August 6, 2025 01:49
wenju-he added a commit to wenju-he/llvm-project that referenced this pull request Aug 6, 2025
…ltin_amdgcn_fence for AMDGPU

It is necessary to add MemorySemantic argument which means the memory or
address space to which the memory ordering is applied.

The MemorySemantic is also necessary for implementing the SPIR-V
MemoryBarrier instruction. Additionally, the implementation of
__clc_mem_fence on Intel GPUs requires the MemorySemantic argument.

Using __builtin_amdgcn_fence for AMDGPU is follow-up of
llvm#151446 (comment)

llvm-diff shows no change to nvptx64--nvidiacl.bc.
wenju-he added a commit that referenced this pull request Sep 1, 2025
…ltin_amdgcn_fence for AMDGPU (#152275)

It is necessary to add MemorySemantic argument for AMDGPU which means
the memory or address space to which the memory ordering is applied.

The MemorySemantic is also necessary for implementing the SPIR-V
MemoryBarrier instruction. Additionally, the implementation of
__clc_mem_fence on Intel GPUs requires the MemorySemantic argument.

Using __builtin_amdgcn_fence for AMDGPU is follow-up of
#151446 (comment)

llvm-diff shows no change to nvptx64--nvidiacl.bc.
llvm-sync bot pushed a commit to arm/arm-toolchain that referenced this pull request Sep 1, 2025
…d use __builtin_amdgcn_fence for AMDGPU (#152275)

It is necessary to add MemorySemantic argument for AMDGPU which means
the memory or address space to which the memory ordering is applied.

The MemorySemantic is also necessary for implementing the SPIR-V
MemoryBarrier instruction. Additionally, the implementation of
__clc_mem_fence on Intel GPUs requires the MemorySemantic argument.

Using __builtin_amdgcn_fence for AMDGPU is follow-up of
llvm/llvm-project#151446 (comment)

llvm-diff shows no change to nvptx64--nvidiacl.bc.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libclc libclc OpenCL library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants