[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