Re: [C++] Mixed scalar-vector operations

[Marc Glisse <marc dot glisse at inria dot fr>]

On Fri, 5 Oct 2012, Jason Merrill wrote:

> On 09/21/2012 02:32 PM, Marc Glisse wrote:
> > +  gcc_assert (TREE_CODE (type0) == VECTOR_TYPE
> > +	      || TREE_CODE (type1) == VECTOR_TYPE);
> > +  switch (code)
> > +    {
> > +      case RSHIFT_EXPR:
> > +      case LSHIFT_EXPR:
> > +	if (TREE_CODE (type0) == INTEGER_TYPE
> > +	    && TREE_CODE (TREE_TYPE (type1)) == INTEGER_TYPE)
>
> Here you're asserting that one of the types is a vector and then assuming 
> that type1 is a vector and type0 is not.  I guess you need to move the 
> swapping code out of the switch.

I didn't write this code, but my understanding is the following. Most 
operations (like PLUS_EXPR) want 2 arguments of the same type. 
[LR]SHIFT_EXPR are special and also accept to have a vector as first 
argument and a scalar as second argument (but not the reverse).

If the first argument is scalar (TREE_CODE (type0) == INTEGER_TYPE), then 
the second must be a vector, and we return that the first argument needs 
converting. Otherwise, we don't perform any conversion (return 
stv_nothing).

cp_build_binary_op has special code for *SHIFT_EXPR for the case of a 
vector and a scalar in this order.

(I don't know why it was decided that *SHIFT_EXPR would be special that 
way, and I don't mind handling it like the other operations if you prefer)

> > +		error_at (loc, "conversion of scalar to vector "
> > +			       "involves truncation");
>
> These errors should print the types involved.  They also need to be 
> suppressed when !(complain & tf_error).

Will do.

> > +              op0 = convert (TREE_TYPE (type1), op0);
> > +              op0 = build_vector_from_val (type1, op0);
>
> I don't see anything in cp_build_binary_op that makes sure that the 
> VECTOR_TYPE is in type1.

These 2 lines are in a switch in the case where scalar_to_vector returned 
stv_firstarg, meaning that the first arg (op0) is a scalar that needs to 
be converted to a vector of the same type as op1. And some lines above, 
there is:

  type1 = TREE_TYPE (op1);


Am I just missing some comments in the code, or is there something wrong?

I think I should at least change the comment on scalar_to_vector from:

/* Convert scalar to vector for the range of operations.  */

to something like:

/* Determine which of the operands, if any, is a scalar that needs to be
   converted to a vector for the range of operations.  */


And add something in scalar_to_vector about the SHIFT_EXPRs.

Thanks for the comments,

--
Marc Glisse