[cfe-dev] [OpenCL patch] Vector logical operations

[Tanya Lattner]

On Jan 13, 2012, at 10:08 PM, Eli Friedman wrote:

> On Fri, Jan 13, 2012 at 9:45 PM, Tanya Lattner <lattner at apple.com> wrote:
>> 
>> On Jan 13, 2012, at 9:10 PM, Eli Friedman wrote:
>> 
>>> On Fri, Jan 13, 2012 at 8:22 PM, Tanya Lattner <lattner at apple.com> wrote:
>>>> 
>>>> On Jan 12, 2012, at 12:33 PM, Eli Friedman wrote:
>>>> 
>>>>> On Thu, Jan 12, 2012 at 3:04 AM, Anton Lokhmotov
>>>>> <Anton.Lokhmotov at arm.com> wrote:
>>>>>> Please review a patch implementing support for vector logical operations in
>>>>>> OpenCL.  It's based on Tanya's patch [1], with Peter's suggestions [2]
>>>>>> applied.
>>>>>> 
>>>>>> Many thanks,
>>>>>> Anton.
>>>>> 
>>>>> +inline QualType Sema::CheckVectorLogicalOperands(ExprResult LHS,
>>>>> ExprResult RHS,
>>>>> 
>>>>> There's no point to marking this inline.
>>>>> 
>>>>> +    } else if (const ExtVectorType *EVT = resultType->getAs<ExtVectorType>()) {
>>>>> +      // Handle vector types.
>>>>> +      // Vector logical not returns the signed variant of the operand type.
>>>>> +      QualType EltTy =  EVT->getElementType();
>>>>> +      EltTy = EltTy->getAs<BuiltinType>()->getSignedVariant(EltTy, Context);
>>>>> +      resultType = Context.getExtVectorType(EltTy, EVT->getNumElements());
>>>>> +      break;
>>>>> 
>>>>> What if EVT is a float4?
>>>>> 
>>>> 
>>>> Yes, this patch is old and doesn't handle this case (or double). I'll correct it with the changes I have in my tree.
>>>> 
>>>>> More generally, we already have code to compute the correct result
>>>>> type for both of these operations in Sema::CheckVectorCompareOperands;
>>>>> please refactor the patch to use it.
>>>> 
>>>> Ok, it may be possible to use CheckVectorCompareOperands(), but float/double will have to be a special case probably before its called.
>>> 
>>> By "it", I was referring to the type-computation code in
>>> Sema::CheckVectorCompareOperands; refactoring that code out would also
>>> be okay.
>>> 
>> 
>> Yeah, I got it after looking at it closer. How about this patch?
>> 
>> I think thats factored better.
> 
> Taking a look at the OpenCL spec, it looks like float4 && float4 is
> allowed.  Is there some reason your patch doesn't allow it?
> 

My patch only supports OpenCL 1.1.

Thanks,
Tanya

> Otherwise, looks fine.
> 
> -Eli