<html>
<head>
<meta http-equiv="content-type" content="text/html; charset=utf-8">
</head>
<body bgcolor="#FFFFFF" text="#000000">
On 12/5/2014 11:20 PM, Colin Riddell wrote:<br>
<blockquote class=" cite" id="mid_5481F067_3060700_codeplay_com"
cite="mid:5481F067.3060700@codeplay.com" type="cite">
<br>
<blockquote class=" cite" id="Cite_5002431" type="cite">float2 =
char2 ? float2 : float2 </blockquote>
Should result in an error as far as my understanding of the spec'
goes. This is because neither explicit nor implicit conversions
between vector types are allowed in OpenCL 1.2 (page 209 - Section
6.2.2 and page 209 - Section 6.2.1 respectively). Oddly enough,
though. For me I don't see the error..
<br>
<br>
<blockquote class=" cite" id="Cite_5991098" type="cite">seems
Clang decided that the ternary expression has type char2. But
from my understanding of the spec, the type should be float2. </blockquote>
<br>
Yes, if anything it should be float2 - irrespective of the rules
against converting vec to vec.
<br>
</blockquote>
<br>
These two seem to be contradicting each other. If the type of the
ternary operator in the above example is float2, then there should
be no error in assigning it to a float2!<br>
<br>
Also my mistake about the error not showing up. The actual example
that I tried was "float2 = char2 ? float : float". Attached the
modified copy that I had created from your original lit test. This
requires your patch to avoid the assertion. I believe this
assignment should not cause any error at all.<br>
<br>
<blockquote class=" cite" id="mid_5481F067_3060700_codeplay_com"
cite="mid:5481F067.3060700@codeplay.com" type="cite">
<blockquote class=" cite" id="Cite_6966883" type="cite">It further
says that the type of the entire expression is derived from the
second and third expression, and nowhere does it mention the
type of the first expression.
<br>
</blockquote>
Yes, the result is derived from either the second or third
expression, however this needs to be implicitly converted to the
result type. The type of the first expression is just used to work
out whether the result should be handled as vector or scalar..
<br>
<br>
<blockquote class=" cite" id="Cite_5832235" type="cite">char2 ?
int : float
<br>
char2 ? int : unsigned
<br>
float2 ? char : unsigned </blockquote>
Are valid, as far as I can see, and their results would depend on
the type of result, not the types of the values on either side of
the colon.
<br>
Expanding your example by providing an assignment would mean that
if assigned to some vector type, the LHS or RHS values would be
expanded to fit the size of that vector and the types would be
implicitly converted to the base type of the result vector.
<br>
For example..
<br>
float3 = char2 ? int: float would result in a float3 for LHS by
expansion of the scalar values to vec3 and implicit conversion of
the int to float. For RHS, expansion of the scalar to vec3 and
no implicit conversion needed because it's already the same type.
<br>
<br>
Do you agree with this?
<br>
</blockquote>
<br>
No, that's not what I see in the spec on page 220: "as long their
types match, or there is a conversion <snip> that can be
applied to one of the expressions to make their types match.
<snip> This resulting matching type is the type of the entire
expression." The spec does not actually say what is this "resulting
matching type", but I would say it is the one with greater rank
(Section 6.2.6) between LHS or RHS (same as ResTy in the code).<br>
<br>
In the same paragraph, the spec also says that if the condition type
is a vector, the behaviour is similar to the "select" builtin. This
means that the vector sizes of LHS and RHS after conversion need to
match the condition size.<br>
<br>
So for each of the expressions above, we first promote LHS or RHS
(but never both) to the "resulting matching type", and then expand
them to vectors that match the condition type. When we try to assign
this to a float3, the expected error is that the vector sizes do not
match. There is no other way to do it, because we can't expand the
vec2 condition to a vec3.<br>
<br>
<blockquote class=" cite" id="mid_5481F067_3060700_codeplay_com"
cite="mid:5481F067.3060700@codeplay.com" type="cite">
From thinking about this, I now see the two lines I proposed are
not enough in order to implement these rules correctly and fix the
current bug.
<br>
<br>
I see what checkConditionalConvertScalarsToVectors() is trying to
do by converting LHS and RHS to vectors, too. However,
subsequent code checks LHSTy and RHSTy, which are QualType values
based on LHS and RHS before their types were converted and someone
has forgotten to update them.<br>
I don't think it's enough to convert these values to vectors and
let the code that follows incorrectly handle the remainder of the
conversion and ternary logic assuming C99.<br>
<br>
I think the bug may be caused by trying to convert the LHS and RHS
to CondTy, rather than ResTy.
<br>
</blockquote>
<br>
Right.<br>
<br>
Sameer.<br>
</body>
</html>