Re: Vector shuffling

[Richard Henderson <rth at redhat dot com>]

On 08/16/2010 12:32 PM, Artem Shinkarov wrote:
>> You should need to use m_type here in casting the arguments.
>> In particular your existing mask could be V4SI with unsigned
>> elements, whereas the builtin takes V4SI with signed elements.
> 
> Looking into the code of ix86_vectorize_builtin_vec_perm, I see that
> the type returned via m_type is almost always the same to TREE_TYPE
> (TREE_TYPE (vec0)) and in your case for V4SI mode there are two
> functions: IX86_BUILTIN_VEC_PERM_V4SI_U and IX86_BUILTIN_VEC_PERM_V4SI
> for unsigned and signed mask. The only change is happening in
> V2DFmode. Could there be any problems with that?
> 
> And mask_type really returns mask element type.

The message at 

  http://gcc.gnu.org/ml/gcc-patches/2010-08/msg01178.html

in the end should supersede many of the comments here.

>> Um, when are you EVER going to see a VECTOR_CST of the wrong size,
>> with the wrong number of elements.  This is very most certainly a
>> bug elsewhere.
> 
> I noticed it quite recently, that nobody ever checked VECTOR_CST for
> consistency, it is fixed about a week ago. But anyway, we have a loop
> over dynamic list, where exit depends on a number, looks suspicious to
> me. And still you can construct such a VECTOR_CST. So why should we
> segfault, when we can just handle it...

Because it's a bug at the origin of the VECTOR_CST.

Perhaps we need an addition to one of the verify_* routines in tree-cfg.c
to find the culprit earlier, but one should never EVER find a VECTOR_CST
without the proper number of elements.

Extra checks here merely make the code more complex for no advantage.

> ... what's wrong with just
> returning false instead of false assertion?

Because it hides a bug in the vectorizer.  Note the different paths
taken for permutations originating in the vectorizer, where we assert
that the values must be correct, and for permutations originating from
the user, where we generate an error.

> Well, the shuffle functionality I introduce is more generic that
> vec_perm we have. So I don't see any problem that expanding
> builtin_shuffle I use vec_perm? Why should user check against vec_perm
> and shuffle if he just wants to shuffle?

I'm not meaning to suggest that __builtin_shuffle handle its inputs
any differently as far as the user is concerned.

What I mean is that tree-vect-generic.c should make use of the existing
target hook when possible, in order to share code between as many targets
as possible.  Indeed, as the follow-up message referenced above suggests,
the existing target support can be re-arranged so as to completely share
the support required by i386 and powerpc.  In the end, almost all of the 
i386 code you add would reside in tree-vect-generic.c.


r~