Anastasia added inline comments.
================
Comment at: lib/Headers/opencl-c.h:16197
+#ifdef cl_intel_device_side_avc_motion_estimation
+#pragma OPENCL EXTENSION cl_intel_device_side_avc_motion_estimation : enable
+
----------------
Anastasia wrote:
> Should we be using:
> #pragma OPENCL EXTENSION my_ext : begin
> then the user can get correct diagnostics that the functions can only be used
> once the extension is enabled.
Would it be better to use `begin`/`end` instead of `enable`/`disable`?
================
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'}}
----------------
AlexeySachkov wrote:
> Anastasia wrote:
> > AlexeySachkov wrote:
> > > Anastasia wrote:
> > > > 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?
> > > > Also can you assign variables of intel_sub_group_avc_mce_payload_t type
> > > > from the same type?
> > >
> > > Yes, such assignment is allowed.
> > >
> > > > Any other restrictions on assignment (i.e. w integer literals)
> > >
> > > All of these types can only be initialized using call to a special
> > > built-ins or using predefined macros like
> > > `CLK_AVC_REF_RESULT_INITIALIZE_INTEL`. Any other assignment should lead
> > > to an error.
> > >
> > > I found that I'm able to assign variable of type
> > > `intel_sub_group_avc_imc_payload_t` to variable of type
> > > `intel_sub_group_avc_mce_payload_t`, so I will update the patch when I
> > > implement such diagnostic message.
> > >
> > > > and operations over these types?
> > > Variables of these types can only be used as return values or arguments
> > > for built-in functions described in the specification. All other
> > > operations are restricted
> > W/o the spec change it's really difficult to review properly. So are you
> > testing 2 groups of types:
> > 1. Init by zero in `bar`?
> > 2. Init by builtin function in `foo`?
> >
> I would like to test:
> 1. foo: init by literal or variable, negative tests
> 2. far: init by other type or assignment between different types, negative
> tests
> 3. bar: init by special initializers from spec, positive tests
Ok, I see. Considering that the restirctions are not in the spec yet. I feel
adding a comment describing initialization/assignment/operation restrictions
for different types would be helpful here.
================
Comment at: test/SemaOpenCL/intel-subgroup-avc-ext-types.cl:16
+
+#define CLK_AVC_IME_RESULT_INITIALIZE_INTEL 0x0
+#define CLK_AVC_REF_RESULT_INITIALIZE_INTEL 0x0
----------------
Is there any point to have all these defines if they are all mapped to 0? I
think using constant literals are easier for testing.
https://reviews.llvm.org/D51484
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
