Anastasia added inline comments.
================
Comment at: include/clang/AST/OperationKinds.def:325
// Convert a zero value for OpenCL queue_t initialization.
CAST_OPERATION(ZeroToOCLQueue)
----------------
I am wondering if we could potentially unify all of those ZeroToOCL* into one
cast type... and also do similar for all of the zero init patterns.
================
Comment at: include/clang/AST/Type.h:6379
+inline bool Type::isOCLIntelSubgroupAVCType() const {
+#define EXT_OPAQUE_TYPE(Name, Id, Ext)
+#define INTEL_SUBGROUP_AVC_TYPE(ExtType, Id) \
----------------
I guess this define is not needed here.
================
Comment at: lib/CodeGen/CGOpenCLRuntime.cpp:68
+ return llvm::PointerType::get( \
+ llvm::StructType::create(Ctx, "opencl." #ExtType), 0);
+#include "clang/Basic/OpenCLExtensionTypes.def"
----------------
I think more generic approach would be to pass AddrSpc and then targets can
override getOpenCLTypeAddrSpace putting address space that is needed.
================
Comment at: lib/Headers/opencl-c.h:16196
+#pragma OPENCL EXTENSION cl_intel_device_side_avc_motion_estimation : begin
+
----------------
I think we should guard this by
#ifdef cl_intel_device_side_avc_motion_estimation
so that it's not added for the targets that don't support this.
Also it might be worth adding a check for a function from this block into
`test/Headers/opencl-c-header.cl` to verify that it's unavailable by default.
================
Comment at: lib/Headers/opencl-c.h:16320
+
+#define CLK_AVC_IME_PAYLOAD_INITIALIZE_INTEL { 0 }
+#define CLK_AVC_REF_PAYLOAD_INITIALIZE_INTEL { 0 }
----------------
Could this just be regular integer literal like the ones above?
================
Comment at: test/SemaOpenCL/intel-subgroup-avc-ext-types.cl:1
+// RUN: %clang_cc1 %s -triple spir-unknown-unknown -cl-std=CL1.2
-cl-ext=+cl_intel_device_side_avc_motion_estimation -fsyntax-only -verify
-DNEGATIVE %s
+// RUN: %clang_cc1 %s -triple spir-unknown-unknown -cl-std=CL1.2
-cl-ext=+cl_intel_device_side_avc_motion_estimation -fsyntax-only -verify %s
----------------
Any reasons to separate into 2 clang calls? Could we tests both in one
invocation of parsing.
================
Comment at: test/SemaOpenCL/intel-subgroup-avc-ext-types.cl:13
+
+#define CLK_AVC_IME_PAYLOAD_INITIALIZE_INTEL { 0 }
+#define CLK_AVC_REF_PAYLOAD_INITIALIZE_INTEL { 0 }
----------------
Just 0 would work too?
================
Comment at: test/SemaOpenCL/intel-subgroup-avc-ext-types.cl:26
+ char4 c4, event_t e, struct st ss) {
+ intel_sub_group_avc_mce_payload_t payload_mce = 0; // No zero initializer
for mce types
+ // expected-error@-1 {{initializing 'intel_sub_group_avc_mce_payload_t' with
an expression of incompatible type 'int'}}
----------------
Would it make sense to add a check for non-zero constant?
Also can you assign variables of intel_sub_group_avc_mce_payload_t type from
the same type? Any other restrictions on assignment (i.e. w integer literals)
and operations over these types?
================
Comment at: test/SemaOpenCL/intel-subgroup-avc-ext-types.cl:54
+ intel_sub_group_avc_ime_dual_reference_streamin_t dstreamin_list = {0x0,
0x1};
+ // expected-warning@-1 {{excess elements in struct initializer}}
+ intel_sub_group_avc_ime_dual_reference_streamin_t dstreamin_list2 = {};
----------------
This error message is probably not the best, but at least it's rejected. Some
thing like
`initializing ... with an expression of incompatible type` would have been
better. Not asking to do this change though...
================
Comment at: test/SemaOpenCL/intel-subgroup-avc-ext-types.cl:57
+ // expected-error@-1 {{scalar initializer cannot be empty}}
+ intel_sub_group_avc_ime_dual_reference_streamin_t dstreamin_list3 = {c};
+ // expected-error@-1 {{initializing
'intel_sub_group_avc_ime_dual_reference_streamin_t' with an expression of
incompatible type 'char'}}
----------------
Can you add something like:
= {1}
https://reviews.llvm.org/D51484
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
