[PATCH] D51484: [OpenCL] Add support of cl_intel_device_side_avc_motion_estimation extension

Alexey Sotkin via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Sep 17 09:44:56 PDT 2018


AlexeySotkin added inline comments.


================
Comment at: include/clang/Basic/OpenCLExtensionTypes.def:27
+
+INTEL_SGAVC_TYPE(mce_payload_t, McePayload)
+INTEL_SGAVC_TYPE(ime_payload_t, ImePayload)
----------------
Anastasia wrote:
> AlexeySachkov wrote:
> > Anastasia wrote:
> > > From the specification of this extension I can't quite see if these types have to be in Clang instead of the header. Can you please elaborate on any example where it wouldn't be possible for this type to be declared in the header using the technique explained in:
> > > https://clang.llvm.org/docs/UsersManual.html#opencl-extensions 
> > We cannot define these types in header because their layout is not defined in specification, i.e. all of these types are opaque
> This is not the reason to add functionality to Clang. You can easily sort such things with target specific headers or even general headers (see `ndrange_t` for example). Spec doesn't have to describe everything. The question is whether there is something about those types that can't be handled using standard include mechanisms. Usually it's prohibited behaviors that can't be represented with such mechanisms. Like if some operations have to be disallowed or allowed (since in OpenCL C you can't define user defined operators) with the types.
> 
> I think the trend is to avoid adding complexity into Clang, unless there is no other way to implement some feature correctly.
Major part of these types must support initialization only by zero. intel_sub_group_avc_mce_payload_t and intel_sub_group_avc_mce_result_t must support initialization only via special builtins defined in the spec. Corresponding errors must be reported. I think we can't implement this behavior using standard include mechanism, can we?

Possible value of the additional complexity, except builtin declaration, is that the patch is quite generic. So next time anyone wants to implement an extension with a type restrictions which can't be handled with the include mechanism, all that they needs to do is to modify this single file.


Repository:
  rC Clang

https://reviews.llvm.org/D51484





More information about the cfe-commits mailing list