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

[Sahasrabuddhe, Sameer]

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.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20141211/0eaa297d/attachment.html>
-------------- next part --------------
      Implementing OpenCL ternary operator with vector operands
      =========================================================

Author: Sameer Sahasrabuddhe <sameer.sahsrabuddhe at amd.com>
Date: 2014-12-11 

The ternary operator (?:) in OpenCL 1.2 has the following form:

  result = exp1 ? exp2 : exp3

The operator first evaluates exp1, and compares its value to zero. If
the value is not equal to zero, then the operator selects to evaluate
exp2, otherwise it evaluates exp3. The situation when one or more of
the three operands is a vector needs clarification.

Three places in the OpenCL 1.2 spec are relevant here:
1. Page 220, which defines the behaviour of the ternary operator.
2. Section 6.2.6, which defines the usual arithmetic operations.
3. Page 268, which defines the select builtin.

In particular, this is an attempt to interpret page 220 in the
broadest possible way:

  "The second and third expressions can be any type, 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 two snips refer to the applicable conversion rules that can be
used to determine the resulting matching type.

The semantics described on page 220 state that when exp1 is a vector,
then the operator is "equivalent to calling the select builtin". The
nature of this equivalence is open to interpretation. In particular,
the following inconsistencies stand out:

1. The select builtin does not compare the condition to zero, but
   instead checks only the MSB of the value. This seems strange
   anyway, because OpenCL 1.2 also defines the boolean "true" value to
   be the integer constant "1". It seems reasonable to resolve this in
   favour of comparing to zero for the ternary operator.

2. If exp1 is a vector and either one or both of exp2 and exp3 are
   scalar, then page 220 leaves room for performing "usual arithmetic
   conversions" on exp2 and exp3.

It seems reasonable to assume that the intended meaning of the
"equivalance" refers two aspects:

1. If exp1 is a vector, then both exp2 and exp3 need to be evaluated,
   since it is to be treated like a function call. This can cause
   surprising results, but it is required for an element-wise
   construction of the result vector as seen below.

2. All operands exp1, exp2, and exp3 are assumed to be vectors of the
   same length (possibly after usual arithmetic conversions), and the
   resulting vector is computed as:

   result[i] = exp1[i] ? exp2[i] : exp3[i] 

   where i is every valid index into the vectors.

Section 6.2.6 "Usual Arithmetic Conversions" seems to say that for a
given operator, either all operands are of matching vector types, or
at most one operand is a vector. This can be overly restrictive for
the ternary operator because exp1 is treated specially as seen below.
Page 220 seems to over-ride this by saying that the result type is
determined only by exp2 and exp3. But this is insufficient, because
exp1 can affect the vector size of the result as seen below.

The three pieces in the OpenCL 1.2 spec could be synthesised into a
consistent behaviour for the ternary operator as follows:

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. Scalar types will be handled by usual arithmetic
      conversions. See point (6.1) below for additional rules.

   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.

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.

Some notable examples (needs fixed width font to view correctly):

  exp1     exp2   exp3        result type   evalutation  
 --------+------+-----------+-------------+-------------
  char     char   unsigned    unsigned      either       
  char     char   unsigned2   unsigned2     either       
  int      char   char        char          either       
  float    char   char        error         -            
  char2    int    float       error         -            
  int2     int    float       float2        both         
  int2     char   char        error         -            
  int2     char   int         int2          both         
  int2     int2   float2      error         -            
  int2     int2   float       error         -            
  int2     int    float2      float2        both         
  float2   char   char        error         -