Anastasia accepted this revision. Anastasia added inline comments. This revision is now accepted and ready to land.
================ Comment at: include/clang/Basic/Builtins.def:1427 +// OpenCL half load/store builtin +BUILTIN(__builtin_store_half, "vdh*", "n") +BUILTIN(__builtin_store_halff, "vfh*", "n") ---------------- jvesely wrote: > Anastasia wrote: > > I think this should be a language builtin (see above) but perhaps we might > > need to extend the language version here. Because I believe we only have > > OpenCL v2.0 currently. > > > > Also this should only be available if `cl_khr_fp16` is supported and > > enabled? I think we are doing similar with some subgroups functions (e.g. > > `get_kernel_sub_group_count_for_ndrange`) that are only supported by > > `cl_khr_subgroup` but those have custom diagnostic though. May be we could > > leave this check out since `half` is not available if `cl_khr_fp16` is not > > enabled anyways. > This is specifically meant to be used when `cl_khr_fp16` is **not** available. > CLC allows using half as storage format and half pointers without the > extension, > vstore_half/vload_half are used to load/store half values. (CL1.2 CH 6.1.1.1) > > These builtins are not necessary if `cl_khr_fp16` is available (we can use > regular loads/stores). > > I'll take stab at making these CLC only, but similarly to device specific > builtins it looked useful beyond that, since these builtins provide access to > half type storage. Strange. This is not how I would interpret from the extension spec though: https://www.khronos.org/registry/OpenCL/sdk/1.2/docs/man/xhtml/cl_khr_fp16.html But I think for this change is probably fine indeed because this doesn't affect half type itself. Repository: rL LLVM https://reviews.llvm.org/D37231 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
