Re: [PATCH] Fix PR64829

[Kyrill Tkachov <kyrylo dot tkachov at arm dot com>]

Hi Richard,

On 28/01/15 14:14, Richard Biener wrote:
> This fixes PR64829 where widening shift pattern detection fails to
> verify the widening operation is used only in the shift.
>
> Bootstrap and regtest running on x86_64-unknown-linux-gnu.

This patch causes a testsuite fail on an arm-none-eabi cross:
FAIL: gcc.dg/vect/vect-widen-shift-u8.c scan-tree-dump-times vect 
"vect_recog_widen_shift_pattern: detected" 2
FAIL: gcc.dg/vect/vect-widen-shift-u8.c -flto -ffat-lto-objects 
scan-tree-dump-times vect "vect_recog_widen_shift_pattern: detected" 2

The widen_shift pattern doesn't get recognised although the loop is 
still vectorised.
Is this something we can avoid?
Or should the test be adjusted?

Thanks,
Kyrill

> Richard.
>
> 2015-01-28  Richard Biener  <rguenther@suse.de>
>
> 	PR tree-optimization/64829
> 	* tree-vect-patterns.c (vect_recog_widen_shift_pattern): Check
> 	that the shift operand has a single use only.
>
> 	* gcc.dg/vect/pr64829.c: New testcase.
>
> Index: gcc/tree-vect-patterns.c
> ===================================================================
> --- gcc/tree-vect-patterns.c	(revision 220205)
> +++ gcc/tree-vect-patterns.c	(working copy)
> @@ -1732,9 +1732,11 @@ vect_recog_widen_shift_pattern (vec<gimp
>     if (TREE_CODE (oprnd0) != SSA_NAME || TREE_CODE (oprnd1) != INTEGER_CST)
>       return NULL;
>   
> -  /* Check operand 0: it has to be defined by a type promotion.  */
> -  if (!type_conversion_p (oprnd0, last_stmt, false, &half_type0, &def_stmt0,
> -                          &promotion)
> +  /* Check operand 0: it has to be defined by a type promotion and it
> +     should be only used by the shift.  */
> +  if (!has_single_use (oprnd0)
> +      || !type_conversion_p (oprnd0, last_stmt, false, &half_type0, &def_stmt0,
> +			     &promotion)
>         || !promotion)
>        return NULL;
>   
> Index: gcc/testsuite/gcc.dg/vect/pr64829.c
> ===================================================================
> --- gcc/testsuite/gcc.dg/vect/pr64829.c	(revision 0)
> +++ gcc/testsuite/gcc.dg/vect/pr64829.c	(working copy)
> @@ -0,0 +1,66 @@
> +/* { dg-do compile } */
> +
> +typedef unsigned char Uint8;
> +typedef int Sint32;
> +typedef unsigned int Uint32;
> +
> +typedef union RMColorDataRef
> +{
> +  Uint8* data8;
> +} RMColorDataRef;
> +
> +typedef struct RMColorData
> +{
> +  Uint32 dataCount;
> +  RMColorDataRef dataRef;
> +} RMColorData;
> +
> +typedef struct RMColorTable
> +{
> +  Uint8 dataCompsOut;
> +  RMColorDataRef dataRef;
> +} RMColorTable;
> +
> +int fail ( const RMColorData * pInColor,
> +	   RMColorData * pOutColor,
> +	   const RMColorTable * pColorTable )
> +{
> +  Uint32 comp;
> +  Uint8 nCompOut;
> +
> +  Sint32 result;
> +
> +  Uint32 interpFrac1, interpFrac2, interpFrac3;
> +  Sint32 val0, val1, val2, val3;
> +
> +  Uint8 * pOut;
> +
> +  const Uint8 * pClutData;
> +  const Uint8 * pCornerPoint0;
> +
> +  Uint8 lastOut[((8) > (4) ? (8) : (4))];
> +
> +  pOut = pOutColor->dataRef.data8;
> +  pClutData = pColorTable->dataRef.data8;
> +
> +  nCompOut = pColorTable->dataCompsOut;
> +
> +  pCornerPoint0 = pClutData;
> +
> +  for (comp = 0; comp < nCompOut; comp++)
> +    {
> +      val0 = *pCornerPoint0++;
> +
> +      result = val0 << 4;
> +
> +      result += (val1 - val0) * interpFrac1;
> +      result += (val2 - val1) * interpFrac2;
> +      result += (val3 - val2) * interpFrac3;
> +
> +      *pOut++ = lastOut[comp] = (Uint8)(result >> 4);
> +    }
> +
> +  return (0);
> +}
> +
> +/* { dg-final { cleanup-tree-dump "vect" } } */