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

[Eli Friedman]

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?

Otherwise, looks fine.

-Eli