[PATCH] Support for floating point vector logical operations

[Tanya Lattner]

On Feb 14, 2013, at 3:29 AM, Joey Gouly <joey.gouly at arm.com> wrote:

> Hi Tanya,
> 
> Thanks for your review.
> 
>> 1) Use, isFPOrFPVectorTy() instead of getScalarType()->isFloatingPointTy()
> Done
> 
>> 2) Why are you removing the newline at the start of the functions?
> Other functions don't have an empty newline there.
> 
>> 3) // OpenCL v1.1 s6.3.f/g/h, add very short description of what the rule
> your are checking is.
> Ok.
> 
>> 4) I'm not sure this patch is complete. I get errors when I run your test
> cases. 
>> 
>> error: 'error' diagnostics seen but not expected: 
>>   Line 27: can't convert between vector values of different size ('float4'
> and 'int')
>>   Line 27: invalid operands to binary expression ('float4' and 'int')
>>   Line 28: can't convert between vector values of different size ('float4'
> and 'int')
>>   Line 28: invalid operands to binary expression ('float4' and 'int')
>>   Line 29: redefinition of 'f4baf'
>>   Line 30: redefinition of 'f4bof'
>> error: 'note' diagnostics seen but not expected: 
>>    Line 24: previous definition is here
>>   Line 25: previous definition is here
>> 8 errors generated.
> 
>> I think you are missing some logic in Sema when checking vector operands.
>  I can integrate this with your changes if you want.
>> Its actually from a previous proposed patch that didn't make it into
> mainline yet. 
> 
> Sorry, bad test. I was going to follow up with a patch  for "can't convert
> between vector values of different size ('float4' and 'int')",
> but if you have it, go ahead after this goes in.
> 

I can add this in a follow up.

>> 5) I think you should add test cases for double as well. 
> Any ideas on a good way to do this, apart from copy-pasting the test and
> s/float/double/ ?
> 

No :)

> Ok to commit?
> 

Yes.

-Tanya

> Thanks,
> Joey<floating_log_ops.diff>