<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body bgcolor="#FFFFFF" text="#000000">
Hello Pekka,<br>
<br>
Thanks for the comments! Attached an updated patch; see replies
below ...<br>
<br>
<div class="moz-cite-prefix">On 1/30/2015 5:00 PM, Pekka
Jääskeläinen wrote:<br>
</div>
<blockquote class=" cite" id="mid_54CB6B51_6070506_tut_fi"
cite="mid:54CB6B51.6070506@tut.fi" type="cite">- to me it's
unintuitive that the check functions return 'false' <br>
if the checked thing is _valid_, but if this is common elsewhere
in <br>
Clang (as it seems), fine <br>
</blockquote>
<br>
Yes, this is common in the same file, and also in other parts. I
find it a bit confusing myself ... one would expect that a
convention of returning true will allow the use of short-circuiting
operators such as "return checkA() && checkB() &&
checkC()", or even "checkA() || die()".<br>
<br>
<blockquote class=" cite" id="mid_54CB6B51_6070506_tut_fi"
cite="mid:54CB6B51.6070506@tut.fi" type="cite">-
OpenCLCheckVectorConditional needs a comment of its return value
and <br>
in the function name seems unintuitive as it's doing more than
checking, <br>
again, if this is a Clang convention, fine <br>
</blockquote>
<br>
Improved the comment to reflect the return value. But again, the
"check" seems to be a convention used by other functions in the same
file.<br>
<br>
<blockquote class=" cite" id="mid_54CB6B51_6070506_tut_fi"
cite="mid:54CB6B51.6070506@tut.fi" type="cite"> - some of the
diagnostics error message vector types have pretty <br>
printed OpenCL C vector datatypes (typedef'd names), some refer
to <br>
the underlying __attribute__'d type (when complaining about func
<br>
return values?) -- is there something that can be done to <br>
make them all print the OpenCL C specs datatype name? <br>
</blockquote>
<br>
I couldn't find an easy way to get the OpenCL name, short of
reconstructing it myself. Note that this issue occurs where Clang
infers a vector type on its own, and there is no way to trace it
back to a typedef. More importantly, such a typedef might not even
exist because Clang may have inferred a type that is not used "by
name" anywhere else in the source. I was reluctant to reconstruct
the type name as a string, mainly because these names have not been
part of Clang; they are always introduced as explicit typedefs.<br>
<br>
The newer patch produces an arguably cleaner description:<br>
<br>
"(vector of 2 'int' values)"<br>
<br>
instead of the original<br>
<br>
"'int __attribute__((ext_vector_type(2)))' (vector of 2 'int'
values)"<br>
<br>
I do not have a strong opinion on which one is more suitable/useful
in an error message. If we really want OpenCL names, then perhaps we
can put implicit typedefs for all OpenCL vector types in
Sema::Initialize(). Then we could provide a way to retrieve them as
QualTypes instead of creating them on the fly.<br>
<br>
Two other updates to the patch:<br>
<ol>
<li>Testcase ntest03 now uses "uchar" arguments. This highlights
the effect of canonically printing the vector type as "(vector
of 2 'unsigned char' values)".</li>
<li>Eliminated two unnecessary arguments "VK" and "OK" from
OpenCLCheckVectorConditional().<br>
</li>
</ol>
Sameer.<br>
<br>
</body>
</html>