<html><head><meta http-equiv="Content-Type" content="text/html charset=windows-1252"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><div>1) Use, <span style="color: rgb(61, 29, 129); font-family: Menlo; font-size: 11px;">isFPOrFPVectorTy</span><span style="font-family: Menlo; font-size: 11px;">() instead of </span>getScalarType()->isFloatingPointTy()</div><div>2) Why are you removing the newline at the start of the functions?</div><div>3) // OpenCL v1.1 s6.3.f/g/h, add very short description of what the rule your are checking is.</div><div>4) I'm not sure this patch is complete. I get errors when I run your test cases. </div><div><br></div><div><div>error: 'error' diagnostics seen but not expected: </div><div> Line 27: can't convert between vector values of different size ('float4' and 'int')</div><div> Line 27: invalid operands to binary expression ('float4' and 'int')</div><div> Line 28: can't convert between vector values of different size ('float4' and 'int')</div><div> Line 28: invalid operands to binary expression ('float4' and 'int')</div><div> Line 29: redefinition of 'f4baf'</div><div> Line 30: redefinition of 'f4bof'</div><div>error: 'note' diagnostics seen but not expected: </div><div> Line 24: previous definition is here</div><div> Line 25: previous definition is here</div><div>8 errors generated.</div></div><div><br></div><div>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. </div><div><br></div><div>5) I think you should add test cases for double as well. </div><div><br></div><div>-Tanya</div><div><br></div><div><br></div><div><br></div><div><br></div><br><div><div>On Feb 8, 2013, at 9:10 AM, Joey Gouly <<a href="mailto:joey.gouly@arm.com">joey.gouly@arm.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite">Hi all!<br><br>This patch adds support for floating point logical operations for OpenCL<br>1.2, and errors for OpenCL 1.1.<br><br>VisitUnaryLNot, VisitBinLAnd and VisitBinLOr were changed to support<br>floating point types.<br><br>In lib/Sema/SemaExpr.cpp, CheckVectorLogicalOperands, there was a line:<br><blockquote type="cite">if (vType.isNull() || vType->isFloatingType())<br></blockquote>I don't think the isFloatingType() could ever be true, since the type from<br>CheckVectorOperands would always be a VectorType?<br>I removed this part, and did a build/test and nothing failed, so I think it<br>was dead code.<br><br>Any comments?<br><br>Thanks,<br>Joey<span><logical_ops.diff></span>_______________________________________________<br>cfe-commits mailing list<br><a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits<br></blockquote></div><br></body></html>