[PATCH] D106343: [OpenCL] Support cl_ext_float_atomics

haonanya added a comment. Hi, svenvh. I am ok with the patch. Thanks very much. I'd appreciate it if you help review https://github.com/KhronosGroup/SPIRV-LLVM-Translator/pull/1116 as well. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106343/new/

[PATCH] D106343: [OpenCL] Support cl_ext_float_atomics

svenvh added a comment. Apologies for the delayed response. In D106343#2967089 , @haonanya wrote: > Hi, svenvh. > Should we use cl_khr_int64_base_atomics and cl_khr_int64_extended_atomics to > guard the functions using atomic_double type? > Thanks very

[PATCH] D106343: [OpenCL] Support cl_ext_float_atomics

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGd353d1c50112: [OpenCL] Support cl_ext_float_atomics (authored by svenvh). Herald added a subscriber: ldrumm. Changed prior to commit: https://revi

[PATCH] D106343: [OpenCL] Support cl_ext_float_atomics

haonanya added a comment. Kindly ping CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106343/new/ https://reviews.llvm.org/D106343 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commi

[PATCH] D106343: [OpenCL] Support cl_ext_float_atomics

haonanya added a comment. In D106343#2967089 , @haonanya wrote: > Hi, svenvh. > Should we use cl_khr_int64_base_atomics and cl_khr_int64_extended_atomics to > guard the functions using atomic_double type? > Thanks very much. > > #if defined(__opencl

[PATCH] D106343: [OpenCL] Support cl_ext_float_atomics

Anastasia added a comment. In D106343#2974055 , @haonanya wrote: > Hi, svenvh and Anastasia. If you approve the patch, could you please submit > it? > I don't have permission to do it. Sure, I can commit it on your behalf, `atomic_half` can be added sep

[PATCH] D106343: [OpenCL] Support cl_ext_float_atomics

haonanya added a comment. Hi, svenvh and Anastasia. If you approve the patch, could you please submit it? I don't have permission to do it. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106343/new/ https://reviews.llvm.org/D106343 ___ cfe-com

[PATCH] D106343: [OpenCL] Support cl_ext_float_atomics

haonanya added a comment. Hi, svenvh. Should we use cl_khr_int64_base_atomics and cl_khr_int64_extended_atomics to guard the functions using atomic_double type? Thanks very much. #if defined(__opencl_c_ext_fp64_local_atomic_min_max) double __ovld atomic_fetch_min(volatile __local atomic_dou

[PATCH] D106343: [OpenCL] Support cl_ext_float_atomics

haonanya added a comment. In D106343#2892946 , @Anastasia wrote: > The extension spec seems to also mention `atomic_half`. Are planning to add > it too? Hi, Anastasia. I also have some work to translate these atomic builtins on SPIRV currently, so I'd

[PATCH] D106343: [OpenCL] Support cl_ext_float_atomics

svenvh accepted this revision. svenvh added a comment. This revision is now accepted and ready to land. LGTM, thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106343/new/ https://reviews.llvm.org/D106343 ___ cfe-commits mailing list cfe-

[PATCH] D106343: [OpenCL] Support cl_ext_float_atomics

haonanya updated this revision to Diff 368256. haonanya added a comment. Fix formatting issues CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106343/new/ https://reviews.llvm.org/D106343 Files: clang/lib/Headers/opencl-c-base.h clang/lib/Headers/opencl-c.h clang/lib/Sema/OpenCLBui

[PATCH] D106343: [OpenCL] Support cl_ext_float_atomics

svenvh added a comment. Thanks for the update! I have a comment about indentation, other than that this is looking good to me. Comment at: clang/lib/Sema/OpenCLBuiltins.td:1129 +let Extension = FuncExtFloatAtomicsFp64GlobalAdd in { +def : Builtin<"atomic_fetch_" # Mod

[PATCH] D106343: [OpenCL] Support cl_ext_float_atomics

haonanya updated this revision to Diff 368119. haonanya added a comment. Unify formatting and fix some errors on OpenCLBuiltins.td CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106343/new/ https://reviews.llvm.org/D106343 Files: clang/lib/Headers/opencl-c-base.h clang/lib/Headers/o

[PATCH] D106343: [OpenCL] Support cl_ext_float_atomics

svenvh added inline comments. Comment at: clang/lib/Sema/OpenCLBuiltins.td:1121 +let Extension = FuncExtFloatAtomicsFp32GlobalAdd in { +def: + Builtin<"atomic_fetch_" #ModOp, Please try to follow the formatting used in the rest of this file: ``` def

[PATCH] D106343: [OpenCL] Support cl_ext_float_atomics

haonanya updated this revision to Diff 367980. haonanya added a comment. Let builtins are guarded by related macro CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106343/new/ https://reviews.llvm.org/D106343 Files: clang/lib/Headers/opencl-c-base.h clang/lib/Headers/opencl-c.h clan

[PATCH] D106343: [OpenCL] Support cl_ext_float_atomics

svenvh added inline comments. Comment at: clang/lib/Sema/OpenCLBuiltins.td:1108 + +let Extension = FuncExtFloatAtomics in { + let MinVersion = CL20 in { So now all of those builtins are guarded by `cl_ext_float_atomics`, which is good, but not by any of the `__

[PATCH] D106343: [OpenCL] Support cl_ext_float_atomics

haonanya updated this revision to Diff 367178. haonanya added a comment. Remove OpenCL3.0 macro guards on opencl-c.h and OpenCLBuiltins.td. Add missing feature macro. simplify test. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106343/new/ https://reviews.llvm.org/D106343 Files: clan

[PATCH] D106343: [OpenCL] Support cl_ext_float_atomics

svenvh added a comment. Thanks for the update! I have a few points to improve the patch. Comment at: clang/lib/Sema/OpenCLBuiltins.td:1107 +// The functionality added by cl_ext_float_atomics extension +let MinVersion = CL30 in { + foreach TypePair = Do we rea

[PATCH] D106343: [OpenCL] Support cl_ext_float_atomics

haonanya added a comment. Hi, Anastasia and svenvh. Sorry for late reply. I have updated patch per your comments. Many thanks for your comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106343/new/ https://reviews.llvm.org/D106343 ___ c

[PATCH] D106343: [OpenCL] Support cl_ext_float_atomics

haonanya updated this revision to Diff 366829. haonanya added a comment. Add the new builtins to clang/lib/Sema/OpenCLBuiltins.td. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106343/new/ https://reviews.llvm.org/D106343 Files: clang/lib/Headers/opencl-c-base.h clang/lib/Headers/o

[PATCH] D106343: [OpenCL] Support cl_ext_float_atomics

svenvh requested changes to this revision. svenvh added inline comments. This revision now requires changes to proceed. Comment at: clang/lib/Headers/opencl-c-base.h:24 #define cl_khr_subgroup_clustered_reduce 1 +#define cl_ext_float_atomics +#ifdef cl_khr_fp16

[PATCH] D106343: [OpenCL] Support cl_ext_float_atomics

Anastasia added inline comments. Comment at: clang/lib/Headers/opencl-c-base.h:24 #define cl_khr_subgroup_clustered_reduce 1 +#define cl_ext_float_atomics +#ifdef cl_khr_fp16 svenvh wrote: > Should this be defined as `1`? > > Should this define be tested in `cl

[PATCH] D106343: [OpenCL] Support cl_ext_float_atomics

svenvh added inline comments. Comment at: clang/lib/Headers/opencl-c-base.h:24 #define cl_khr_subgroup_clustered_reduce 1 +#define cl_ext_float_atomics +#ifdef cl_khr_fp16 Should this be defined as `1`? Should this define be tested in `clang/test/Headers/opencl

[PATCH] D106343: [OpenCL] Support cl_ext_float_atomics

Anastasia added a comment. Just to make sure you are aware Clang doesn't use this header by default, so the upstream users won't be able to call those functions unless you add them into `OpenCLBuiltins.td`: https://clang.llvm.org/docs/OpenCLSupport.html#opencl-builtins This header is only acces