[PATCH] D37231: Add half load and store builtins
Anastasia Stulova via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Fri Sep 1 09:34:48 PDT 2017
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
More information about the cfe-commits
mailing list