<html>
<head>
<meta http-equiv="content-type" content="text/html; charset=utf-8">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<br>
<div class="moz-cite-prefix">On 12/11/2014 5:47 PM, Colin Riddell
wrote:<br>
</div>
<blockquote class=" cite" id="mid_54898B51_5060702_codeplay_com"
cite="mid:54898B51.5060702@codeplay.com" type="cite">
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
Yes, this is a good exercise for me as well. It's really forcing
me to question my understanding, rather than just hacking
something together to make it work. I agree we need more tests, so
I will work on a larger set of both positive and negative tests
once we have solidified our understanding.<br>
</blockquote>
<br>
Good to know that. It definitely affects AMD's frontend and I am
guessing that it also affects the Khronos SPIR generator.<br>
<br>
<blockquote class=" cite" id="mid_54898B51_5060702_codeplay_com"
cite="mid:54898B51.5060702@codeplay.com" type="cite">
<div class="moz-cite-prefix"> I reviewed your document and moved
it around slightly to fit with my understanding of the spec. In
summary, my changes are:<br>
</div>
</blockquote>
<br>
Thanks for looking into it! I have replied below by copying relevant
parts from the file.<br>
<br>
<blockquote class=" cite" id="mid_54898B51_5060702_codeplay_com"
cite="mid:54898B51.5060702@codeplay.com" type="cite">
<div class="moz-cite-prefix"> * Select rules about bits can be
simplified down to something very succinct like: 'use select
when exp1 result is a vector. When using select, exp2 and exp3
must match.' (and conversion rules apply to make them match
detailed in 4.2). This is because, beyond checking these types,
at this stage in the compilation we have no other control..
which is what the checkConditionalConvertScalarsToVectors()
function is attempting to do (though this may be incorrect).<br>
<br>
I think these changes are more around the order and organization
of the rules, rather than the actual rules themselves and think
we are starting to come to agreement on them. Let me know what
you think of my changes to your document..<br>
</div>
</blockquote>
<br>
Sure, but one motivation for writing this document was to identify
gray areas, where Clang is following conventions in the absence of
rules. See below regarding point (6) from my original writeup.<br>
<br>
Pasting some of the lines from your attached file:<br>
<br>
<blockquote type="cite"> 1. exp2 and exp3 can be any type, as
long as they match<br>
<br>
2. If exp2 and exp3 don't match, then one of either exp2 or
exp3<br>
can be converted using the arithmetic conversion rules
for vectors<br>
("both" rules, in your wording): <br>
1. If one is a vector and the other a scalar, the
scalar<br>
may be converted to the base type, and expanded to
the vec <br>
type.<br>
2. If both are vectors of the same type and size, no
implicit<br>
conversion is required.<br>
3. If both are vectors of different type or size, no
implicit<br>
conversion is ALLOWED<br>
3. For *scalars*, the result type is obtained by implicit <br>
conversion on exp2 *or* exp3. (section 6.2.1 of CL Spec).
Which <br>
comes under the "either" rules, by your wording.<br>
</blockquote>
<br>
We don't actually need to say all this. It's enough to just cite
Section 6.2.6. This was my intention in point (5):<br>
<br>
<blockquote type="cite">-5. The result type is obtained by usual
arithmetic conversions on exp2<br>
- and exp3, as specified in Section 6.2.6.</blockquote>
<br>
Aside: Does the minus (-) sign mean that you think this should be
deleted? <br>
<br>
Also, I think that Section 6.2.1 is not really necessary in this
situation, the way it occurs on page 220 of the spec:<br>
<br>
... or there is a conversion in section 6.2.1 Implicit
Conversions that<br>
can be applied to one of the expressions to make their types
match, ...<br>
<br>
Which expression should be implicitly converted? As far as I can
make out, there was no need to mention Section 6.2.1 at this point
in the spec itself, especially because Section 6.2.6 is meant to
clarify exactly this choice: "The purpose is to determine a common
real type for the operands and result.". This section cites the C99
spec when both operands are scalars: "Otherwise, if all operands are
scalar, the usual arithmetic conversions apply, per section 6.3.1.8
of the C99 standard."<br>
<br>
<blockquote type="cite">(I suppose, technically they are both
evaulated, but one converted to the <br>
other. Covered in 4)<br>
- 2. Both exp2 and exp3 are evaluated.</blockquote>
<br>
Yeah, if we have already said that the vector operator behaves like
the select function call, then it implies that both are evaluated.
But I thought it would be good to make this an explicit point
because it was the first thing that tripped me up when I first
encountered the ternary operator long ago! :)<br>
<br>
<blockquote type="cite">- 6. If exp1 is a vector, and the result
type is a scalar, then</blockquote>
<br>
This is an important point; it covers exactly the part that caused
the original bug. By "result type", I meant the one obtained by
usual arithmetic conversions on LHS and RHS. It is the same as ResTy
in the code. If we had stopped at saying that the ternary operator
behaves like the select function, then it would be illegal to have
an expression like "int2 ? float : float". The vector expansion
required is not specified anywhere --- we want to promote ResTy
(scalar float) to a vector that matches the length of CondTy, but we
do not want to change its element type. <br>
<ol>
<li>CondTy is vector, RHS is scalar and LHS is scalar.</li>
<li>Current behaviour: Use usual arithmetic conversions to cast
both RHS and LHS to CondTy. Very buggy because it performs
illegal conversions from scalar to vector.</li>
<li>Expected behaviour: Use usual arithmetic conversions to
determine ResTy from RHS and LHS, then expand it to match CondTy
in length. Actually, only one of RHS and LHS will be casted.<br>
</li>
</ol>
Going by the letter of the spec, it is difficult to say whether the
situation itself is illegal or undefined. Note that a similar
situation with the select builtin will result in a signature
mismatch. Clang currently implements the first version, but at least
you and I agree that the second version is more useful. There is no
reason to say that one behaviour is correct and the other is not.<br>
<br>
Sameer.<br>
<br>
</body>
</html>