Re: PATCH: PR target/42542: Vectorizer produces incorrect results on max of signed intergers

[Uros Bizjak <ubizjak at gmail dot com>]

On Wed, Dec 30, 2009 at 4:48 PM, H.J. Lu <hongjiu.lu@intel.com> wrote:

> x86 integer vector subtraction insns don't set EFLAGS and there is no
> parallel unsigned V4SI saturating subtraction. We can't easily detect
> unsigned underflow. We can only enable umaxv4si3 and uminv4si3 for
> SSE4.1 and XOP. ?Tested on Linux/Intel Core i7. ?OK for trunk and 4.3/4.4?

I see no other way to fixup the underflow. Some comments below:

> ? [(set (match_operand:SSEMODE24 0 "register_operand" "")
> ? ? ? ?(umin:SSEMODE24 (match_operand:SSEMODE24 1 "register_operand" "")
> ? ? ? ? ? ? ? ? ? ? ? ?(match_operand:SSEMODE24 2 "register_operand" "")))]
> - ?"TARGET_SSE2"
> + ?"(TARGET_SSE2 && <MODE>mode == V8HImode)
> + ? || TARGET_SSE4_1
> + ? || TARGET_XOP"
> ?{
> ? if (TARGET_SSE4_1)
> ? ? ix86_fixup_binary_operands_no_copy (UMIN, <MODE>mode, operands);

Please split above pattern into two non-macroized patterns using V8HI
and V4SI modes.

> diff --git a/gcc/testsuite/gcc.dg/vect/vect-reduc-1.c b/gcc/testsuite/gcc.dg/vect/vect-reduc-1.c
> index dbb154d..8da7fb0 100644
> --- a/gcc/testsuite/gcc.dg/vect/vect-reduc-1.c
> +++ b/gcc/testsuite/gcc.dg/vect/vect-reduc-1.c
> @@ -52,5 +52,5 @@ int main (void)
> ? return 0;
> ?}
>
> -/* { dg-final { scan-tree-dump-times "vectorized 3 loops" 1 "vect" { xfail { vect_no_int_add || vect_no_int_max } } } } */
> +/* { dg-final { if { ![ istarget i?86-*-* ] && ![ istarget x86_64-*-* ] } { scan-tree-dump-times "vectorized 3 loops" 1 "vect" { xfail { vect_no_int_add || vect_no_int_max } } } } } */
> ?/* { dg-final { cleanup-tree-dump "vect" } } */

Please also split i.e. vect_no_uint_max (and probably some kind of
vect_no_ushort_max) from vect_no_int_max procedure (defined in
testsuite/lib/target_supports.exp) and use them where appropriate in
vectorizer testsuite.

The patch is OK with these changes, but you will need separate
approval for generic, non-target dependant testsuite changes. Please
wait some time before the patch is backported to 4.4. and 4.3.

Thanks,
Uros.