Re: [RFC/RFT] Tree-level lowering of generic vectors (take 2)

[Richard Henderson <rth at redhat dot com>]

I'm gonna want this patch split up into pieces.

> 	* c-typeck.c (build_binary_op): Do not use RDIV_EXPR for
> 	integer vectors.

Ok.

> 	* convert.c (convert_to_integer): Use vector types' TYPE_SIZE.
> 	(convert_to_vector): Likewise.

Ok, and everything else that just checks TYPE_SIZE instead of mode.

*************** store_constructor
> +      enum machine_mode mode = TYPE_MODE (elttype);
...
> +	  enum machine_mode mode = GET_MODE (target);

Ug.  Sometimes I think we ought to have -Wshadow enabled.  I wonder
how much trouble that would get us in?

My recommendation is to use "eltmode" for the first, since that'll make 

!       if (mode == BLKmode)
!       abort ();

more obviously correct.

Ok to store_constructor changes.

> --- gcc/machmode.def	6 Nov 2003 08:38:50 -0000	1.27
> +++ gcc/machmode.def	19 Jul 2004 14:12:49 -0000
> @@ -186,36 +186,6 @@ CC_MODE (CC);
>  COMPLEX_MODES (INT);
>  COMPLEX_MODES (FLOAT);
>  
> -/* Vector modes.  */

Vector mode movement should be done as a separate patch, last.

> +optab_for_tree_code (enum tree_code code, tree type)
...
> +  trapv = flag_trapv && INTEGRAL_TYPE_P (type) && TYPE_UNSIGNED (type);

	!TYPE_UNSIGNED.

> +char have_regs_of_mode [MAX_MACHINE_MODE];

bool.

> +/* Build a constant of type TYPE, made of VALUE's bits replicated
> +   to fit TYPE's size.  */
> +static tree
> +build_replicated_const (tree type, tree inner_type, HOST_WIDE_INT value)

Description should be "... to fit INNER_TYPE's size"?

> +  int width = TYPE_PRECISION (inner_type);

I believe you want TYPE_SIZE here, otherwise vectors of enum or bool
types will not work as expected.  Not that I believe that works as
people expect anyway, but no need to make things worse.

> +  mask = ((HOST_WIDE_INT)1 << width) - 1;
> +  low = (unsigned HOST_WIDE_INT) ~0 / mask * (value & mask);

Does not work if width == HOST_BITS_PER_WIDE_INT, which is why I
specifically did not use this construct in the example I gave you.

> +  /* All powers of two <= 32 give a different result modulo 37.  */

Comment out of date.

> +  last = type_hash_canon (nunits, build_vector_type (innertype, nunits));

"nunits" is a horrible hash code.  This isn't your fault, I think,
because this entire set of functions are totally borked.  Sigh.

> +  HOST_WIDE_INT max;
...
> +  low_bits = build_replicated_const (word_type, inner_type, max >> 1);

Max needs to be unsigned for this to work as you expect.

> +  op = optab_for_tree_code (code == NEGATE_EXPR ? MINUS_EXPR : code, type);

Using minus instead of negate gets the wrong answer for floating point.

We do have negv4sf2, on i386 at least; others can be updated, or we can
implement the note in i386.md:

  ;; ??? Should probably be done by generic code instead.

since we generate "x ^ { 0x80000000 x width }".  And note that the 
constant there is derivable from real_format_for_mode[mode].signbit.

> +  compute_mode = TYPE_MODE (compute_type);
> +  if (compute_type == type)
> +    {
> +      if ((GET_MODE_CLASS (compute_mode) == MODE_VECTOR_INT
> +	   || GET_MODE_CLASS (compute_mode) == MODE_VECTOR_FLOAT)
> +	  && op
> +	  && op->handlers[compute_mode].insn_code != CODE_FOR_nothing)
> +	{
> +	  /* If there is an handler in the optab, we have nothing to do
> +	     unless we are breaking a BLKmode vector into smaller pieces.  */
> +	  if (compute_type == type)
> +	    return;

Duplicate tests in innermost and outermost if's.

> +	  /* When not optimizing, only lower complex arithmetic.  This is so
> +	     that the loop optimizers do not waste time on lowered vectors...

You mean silly things like moving invariant code out of loops and the like?

> +	     ... and more importantly they can create generic vector code.

And what, exactly, would be wrong with rerunning the pass iff the loop
code did decide to generate vector operations?

Leave this bit out of the patch for now.

> +/* Create a new vector type node holding SUBPARTS units of type INNERTYPE,
> +   and mapped to the machine mode MODE.  Initialize its fields and build
> +   the information necessary for debugging output.  */
>  
> +static tree
> +make_vector_type (tree innertype, int nunits, enum machine_mode mode)

Comment doesn't match the code.

> +  TYPE_MODE (t) = mode;
> +  TYPE_UNSIGNED (t) = TYPE_UNSIGNED (innertype);
> +  TYPE_SIZE_UNIT (t) = int_const_binop (MULT_EXPR, TYPE_SIZE_UNIT (innertype),
> +					nunits_tree, 0);
> +  TYPE_SIZE (t) = int_const_binop (MULT_EXPR, TYPE_SIZE (innertype),
> +				   nunits_tree, 0);

Why are you settting mode and size yourself?  That's layout_type's job.


r~