[cfe-dev] [PATCH] [OpenCL] Conversions for ternary operations between scalar and vector

Colin Riddell colinriddell at codeplay.com
Thu Dec 11 04:17:21 PST 2014


Hi Sameer,

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.

I reviewed your document and moved it around slightly to fit with my 
understanding of the spec. In summary, my changes are:

  * 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).

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..


Regards,
Colin

On 11/12/2014 06:57, Sahasrabuddhe, Sameer wrote:
>
> On 12/10/2014 9:26 PM, Colin Riddell wrote:
>>
>>> 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.
>> Agreed, and thanks for the test. Added that to the patch.
>
> Erm ... turns out that my test is wrong for a different reason! The 
> expression "char2 ? float : float" is not compatible with the select 
> builtin because the number of bits in the element types needs to be 
> the same. I am guessing that this bitness issue might have been the 
> original inspiration for casting to CondTy instead of ResTy, that you 
> fixed. But your fix is correct within its context.
>
>> It's worth pointing out that the select generation is handled in 
>> CGExprScalar.cpp Value *ScalarExprEmitter::
>> VisitAbstractConditionalOperator()
>
> Thanks! That actually saved me from embarking on a spelunking 
> expedition! :D
>
>> Attached is another patch for review, including both tests. Now 
>> ignore the first patch I submitted. This new change passes all the 
>> tests run with check-clang.
>
> There could be more to this than the existing tests. Please see the 
> attached document where I have described my understanding of how the 
> ternary operator is supposed to work. Would appreciate any faults that 
> you find! I hope it is not overkill, but the effort was already useful 
> --- it helped me discover that my testcase above is invalid. Another 
> example which we discussed in previous emails is also wrong: "float ? 
> char : unsigned", because the condition cannot be a floating point type.
>
> Sameer.
>


-- 
Colin Riddell

Team Lead - GPGPU Software Systems

Codeplay Software Ltd
45 York Place, Edinburgh, EH1 3HP
Tel: 0131 466 0503
Fax: 0131 557 6600
Website: http://www.codeplay.com

This email and any attachments may contain confidential and /or privileged information and  is for use  by the addressee only. If you are not the intended recipient, please notify Codeplay Software Ltd immediately and delete the message from your computer. You may not copy or forward it,or use or disclose its contents to any other person. Any views or other information in this message which do not relate to our business are not authorized by Codeplay software Ltd, nor does this message form part of any contract unless so stated.
As internet communications are capable of data corruption Codeplay Software Ltd does not accept any responsibility for any changes made to this message after it was sent. Please note that Codeplay Software Ltd does not accept any liability or responsibility for viruses and it is your responsibility to scan any attachments.
Company registered in England and Wales, number: 04567874
Registered office: 81 Linkfield Street, Redhill RH1 6BY

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20141211/f77a0437/attachment.html>
-------------- next part --------------
1. If one or more operands are vectors, then their lengths must match.

2. The element type of exp1 cannot be a floating point type.

3. If exp1 is a scalar:

   1. No restrictions apply to the types of exp2 and exp3 with respect
      to exp1.

   2. Either exp2 or exp3 is evaluated depending on the value of exp1.

   3. If the result type is a vector, there is no need to expand exp1.

4. If exp1 is a vector:

   1. The number of bits in the element types of all vector types must
      match. This will be handled elsewhere, and is out of our control 
      at the point where we check the types of the expressions in the 
      ternary. All we need to do in SemaExpr.cpp is to check the types and
      ensure that exp2 and exp3 are the same in order to satisfy the 
      rules for select and handle appropriately.
      Therefore, select() rules apply, which are:

      1. exp2 and exp3 can be any type, as long as they match

      2. If exp2 and exp3 don't match, then one of either exp2 or exp3
         can be converted using the arithmetic conversion rules for vectors
         ("both" rules, in your wording): 
             1. If one is a vector and the other a scalar, the scalar
                may be converted to the base type, and expanded to the vec 
                type.
             2. If both are vectors of the same type and size, no implicit
                conversion is required.
             3. If both are vectors of different type or size, no implicit
                conversion is ALLOWED
      3. For *scalars*, the result type is obtained by implicit 
         conversion on exp2 *or* exp3. (section 6.2.1 of CL Spec). Which 
         comes under the "either" rules, by your wording.

(I suppose, technically they are both evaulated, but one converted to the 
other. Covered in 4)
-   2. Both exp2 and exp3 are evaluated.

-5. The result type is obtained by usual arithmetic conversions on exp2
-   and exp3, as specified in Section 6.2.6.


 (In this section, when talking abotu the "result type", are you 
 reffering to the type of either exp2 or exp3? If so, modified section
 4 in  this document covers this)

- 6. If exp1 is a vector, and the result type is a scalar, then
- 
-    1. The scalar result type must have the same number of bits as the
-       element type in exp1, as required by the select builtin. Page
-       220 does not provide for further changing either the result type
-       or exp1 to match the number of bits.
- 
-    2. The scalar result type is expanded to a vector having the same
-       length as exp1. The spec does not actually say this, and it
-       could be considered undefined behaviour. It is a useful but
-       tricky mix-and-match between all three referred sections to
-       arrive at a "reasonable behaviour" for the ternary operator.
-       This is how Clang does it, anyway.


More information about the cfe-dev mailing list