[PATCH] Support for floating point vector logical operations

Joey Gouly joey.gouly at arm.com
Thu Feb 14 03:29:07 PST 2013


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.

> 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/ ?

Ok to commit?

Thanks,
Joey
-------------- next part --------------
A non-text attachment was scrubbed...
Name: floating_log_ops.diff
Type: application/octet-stream
Size: 14742 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130214/e10cacb1/attachment.obj>


More information about the cfe-commits mailing list