Re: [PATCH][RFC] Fix PR39943, wrong code with vectorized conversion on x86_64

[Ira Rosen <IRAR at il dot ibm dot com>]

Richard Guenther <rguenther@suse.de> wrote on 28/04/2009 18:21:58:

>
> This fixes the ICE on 177.mesa caused by type checking which catched
> the wrong-code issue that we are happily doing vectorized
> float -> unsigned and unsigned -> float conversions with the SSE
> cvtdq2ps and cvttps2dq instructions which only work with signed
> integer value ranges.  Oops.
>
> The following patch disables that.  Instead we could in theory
> (with -funsafe-math-optimizations) bias the input value, convert
> and then unbias it again.  No idea if that is faster than not
> vectorizing.  It would need another builtin function and adjustments
> to the cost model.
>
> This patch will cause (at least)
>
> FAIL: gcc.dg/vect/slp-10.c scan-tree-dump-times vect "vectorized 3 loops"

> 1
> FAIL: gcc.dg/vect/slp-10.c scan-tree-dump-times vect "vectorizing stmts
> using SLP" 3
> FAIL: gcc.dg/vect/slp-11.c scan-tree-dump-times vect "vectorized 3 loops"

> 1
> FAIL: gcc.dg/vect/slp-12b.c scan-tree-dump-times vect "vectorized 1
loops"
> 1
> FAIL: gcc.dg/vect/slp-12b.c scan-tree-dump-times vect "vectorizing stmts
> using SLP" 1
> FAIL: gcc.dg/vect/slp-33.c scan-tree-dump-times vect "vectorized 3 loops"

> 1
> FAIL: gcc.dg/vect/slp-33.c scan-tree-dump-times vect "vectorizing stmts
> using SLP" 3
>
> because they all use conversions to/from unsigned ints.  Ira - do you
> prefer adding another vector capability, vect_uintfloat_cvt, here
> or placing strategic XFAILs in the tests?  Are there corresponding
> tests for signed int conversions?  Do ppc and ia64 really handle this
> cases correctly?
>

vect_uintfloat_cvt change is fine with me.
The tests gcc.dg/vect/vect-intfloat-conversion-1/2/3.c test signed int->
float conversion.>
PowerPC determines which instruction to use according to the sign of the
int argument (in function rs6000_builtin_conversion()).

Thanks,
Ira


> I'll bootstrap / test this soon.  Uros, is the backend change ok?
>
> Thanks,
> Richard.
>
> 2009-04-28  Richard Guenther  <rguenther@suse.de>
>
>    PR target/39943
>    * config/i386/i386.c (ix86_vectorize_builtin_conversion): Only
>    allow conversion to signed integers.
>
>    * gcc.c-torture/compile/pr39943.c: New testcase.
>
> Index: gcc/config/i386/i386.c
> ===================================================================
> *** gcc/config/i386/i386.c   (revision 146887)
> --- gcc/config/i386/i386.c   (working copy)
> *************** ix86_veclibabi_acml (enum built_in_funct
> *** 25639,25645 ****
>   static tree
>   ix86_vectorize_builtin_conversion (unsigned int code, tree type)
>   {
> !   if (TREE_CODE (type) != VECTOR_TYPE)
>       return NULL_TREE;
>
>     switch (code)
> --- 25639,25647 ----
>   static tree
>   ix86_vectorize_builtin_conversion (unsigned int code, tree type)
>   {
> !   if (TREE_CODE (type) != VECTOR_TYPE
> !       /* There are only conversions from/to signed integers.  */
> !       || TYPE_UNSIGNED (TREE_TYPE (type)))
>       return NULL_TREE;
>
>     switch (code)
> Index: gcc/testsuite/gcc.c-torture/compile/pr39943.c
> ===================================================================
> *** gcc/testsuite/gcc.c-torture/compile/pr39943.c   (revision 0)
> --- gcc/testsuite/gcc.c-torture/compile/pr39943.c   (revision 0)
> ***************
> *** 0 ****
> --- 1,7 ----
> + void gl_fog_index_pixels(float f, unsigned int n, unsigned int index[])
> + {
> +   unsigned int i;
> +   for (i=0; i<n; i++)
> +     index[i] = (unsigned int) ((float) index[i] + (1.0F-f));
> + }
> +