Patchwork [wide-int] Fix fold_ternary VEC_PERM_EXPR handling

login
register
mail settings
Submitter Richard Sandiford
Date Nov. 2, 2013, 11:14 a.m.
Message ID <87txfvjbse.fsf@talisman.default>
Download mbox | patch
Permalink /patch/287972/
State New
Headers show

Comments

Richard Sandiford - Nov. 2, 2013, 11:14 a.m.
The wide-int conversion for the fold_ternary VEC_PERM_EXPR case
converted a mask of valid element numbers to an element count,
which is one greater.  The old code set need_mask_canon if an index
was greater than the mask, but the new code sets it if an index
is greater than the element count, giving an off-by-one error.

This patch restores the mainline mask handling and uses that in
the gtu_p call instead.

Tested on powerpc64-linux-gnu and x86_64-linux-gnu.  It fixes some
unwanted testsuite differences for one of the ARM targets.  OK to install?

Thanks,
Richard
Kenneth Zadeck - Nov. 2, 2013, 2:01 p.m.
On 11/02/2013 07:14 AM, Richard Sandiford wrote:
> The wide-int conversion for the fold_ternary VEC_PERM_EXPR case
> converted a mask of valid element numbers to an element count,
> which is one greater.  The old code set need_mask_canon if an index
> was greater than the mask, but the new code sets it if an index
> is greater than the element count, giving an off-by-one error.
>
> This patch restores the mainline mask handling and uses that in
> the gtu_p call instead.
>
> Tested on powerpc64-linux-gnu and x86_64-linux-gnu.  It fixes some
> unwanted testsuite differences for one of the ARM targets.  OK to install?
>
> Thanks,
> Richard
looks ok to me.
>
> Index: gcc/fold-const.c
> ===================================================================
> --- gcc/fold-const.c	2013-11-02 11:07:33.097149207 +0000
> +++ gcc/fold-const.c	2013-11-02 11:07:38.107188425 +0000
> @@ -14360,7 +14360,7 @@ fold_ternary_loc (location_t loc, enum t
>       case VEC_PERM_EXPR:
>         if (TREE_CODE (arg2) == VECTOR_CST)
>   	{
> -	  unsigned int nelts = TYPE_VECTOR_SUBPARTS (type), i;
> +	  unsigned int nelts = TYPE_VECTOR_SUBPARTS (type), i, mask;
>   	  unsigned char *sel = XALLOCAVEC (unsigned char, nelts);
>   	  bool need_mask_canon = false;
>   	  bool all_in_vec0 = true;
> @@ -14368,23 +14368,22 @@ fold_ternary_loc (location_t loc, enum t
>   	  bool maybe_identity = true;
>   	  bool single_arg = (op0 == op1);
>   	  bool changed = false;
> -	  int nelts_cnt = single_arg ? nelts : nelts * 2;
>   
> +	  mask = single_arg ? (nelts - 1) : (2 * nelts - 1);
>   	  gcc_assert (nelts == VECTOR_CST_NELTS (arg2));
>   	  for (i = 0; i < nelts; i++)
>   	    {
>   	      tree val = VECTOR_CST_ELT (arg2, i);
> -
>   	      if (TREE_CODE (val) != INTEGER_CST)
>   		return NULL_TREE;
>   
>   	      /* Make sure that the perm value is in an acceptable
>   		 range.  */
>   	      wide_int t = val;
> -	      if (wi::gtu_p (t, nelts_cnt))
> +	      if (wi::gtu_p (t, mask))
>   		{
>   		  need_mask_canon = true;
> -		  sel[i] = t.to_uhwi () & (nelts_cnt - 1);
> +		  sel[i] = t.to_uhwi () & mask;
>   		}
>   	      else
>   		sel[i] = t.to_uhwi ();

Patch

Index: gcc/fold-const.c
===================================================================
--- gcc/fold-const.c	2013-11-02 11:07:33.097149207 +0000
+++ gcc/fold-const.c	2013-11-02 11:07:38.107188425 +0000
@@ -14360,7 +14360,7 @@  fold_ternary_loc (location_t loc, enum t
     case VEC_PERM_EXPR:
       if (TREE_CODE (arg2) == VECTOR_CST)
 	{
-	  unsigned int nelts = TYPE_VECTOR_SUBPARTS (type), i;
+	  unsigned int nelts = TYPE_VECTOR_SUBPARTS (type), i, mask;
 	  unsigned char *sel = XALLOCAVEC (unsigned char, nelts);
 	  bool need_mask_canon = false;
 	  bool all_in_vec0 = true;
@@ -14368,23 +14368,22 @@  fold_ternary_loc (location_t loc, enum t
 	  bool maybe_identity = true;
 	  bool single_arg = (op0 == op1);
 	  bool changed = false;
-	  int nelts_cnt = single_arg ? nelts : nelts * 2;
 
+	  mask = single_arg ? (nelts - 1) : (2 * nelts - 1);
 	  gcc_assert (nelts == VECTOR_CST_NELTS (arg2));
 	  for (i = 0; i < nelts; i++)
 	    {
 	      tree val = VECTOR_CST_ELT (arg2, i);
-
 	      if (TREE_CODE (val) != INTEGER_CST)
 		return NULL_TREE;
 
 	      /* Make sure that the perm value is in an acceptable
 		 range.  */
 	      wide_int t = val;
-	      if (wi::gtu_p (t, nelts_cnt))
+	      if (wi::gtu_p (t, mask))
 		{
 		  need_mask_canon = true;
-		  sel[i] = t.to_uhwi () & (nelts_cnt - 1);
+		  sel[i] = t.to_uhwi () & mask;
 		}
 	      else
 		sel[i] = t.to_uhwi ();