Patchwork bit_field_ref of constructor of vectors

login
register
mail settings
Submitter Marc Glisse
Date Nov. 12, 2012, 1:31 p.m.
Message ID <alpine.DEB.2.02.1211121408210.8152@stedding.saclay.inria.fr>
Download mbox | patch
Permalink /patch/198389/
State New
Headers show

Comments

Marc Glisse - Nov. 12, 2012, 1:31 p.m.
Hello,

this patch lets us fold a bit_field_ref of a constructor even when the 
elements are vectors. Writing a testcase is not that convenient because of 
the lack of a dead code elimination pass after forwprop4 (RTL doesn't 
always remove everything either), but I see a clear difference on this 
test on x86_64 without AVX.

typedef double vec __attribute__((vector_size(4*sizeof(double))));
void f(vec*x){
   *x+=*x+*x;
}
double g(vec*x){
   return (*x+*x)[3];
}
void h(vec*x, double a, double b, double c, double d){
   vec y={a,b,c,d};
   *x+=y;
}


I don't know if the patch (assuming it is ok) is suitable for stage 3, it 
may have to wait. It passes bootstrap+testsuite.

I am still not quite sure why we even have a valid_gimple_rhs_p function 
(after which we usually give up if it says no) instead of gimplifying, so 
I may have missed a reason why CONSTRUCTOR or BIT_FIELD_REF shouldn't be 
ok.


2012-11-12  Marc Glisse  <marc.glisse@inria.fr>

 	* fold-const.c (fold_ternary_loc) [BIT_FIELD_REF]: Handle
 	CONSTRUCTOR with vector elements.
 	* tree-ssa-propagate.c (valid_gimple_rhs_p): Handle CONSTRUCTOR
 	and BIT_FIELD_REF.
Richard Guenther - Nov. 27, 2012, 1:17 p.m.
On Mon, Nov 12, 2012 at 2:31 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
> Hello,
>
> this patch lets us fold a bit_field_ref of a constructor even when the
> elements are vectors. Writing a testcase is not that convenient because of
> the lack of a dead code elimination pass after forwprop4 (RTL doesn't always
> remove everything either), but I see a clear difference on this test on
> x86_64 without AVX.
>
> typedef double vec __attribute__((vector_size(4*sizeof(double))));
> void f(vec*x){
>   *x+=*x+*x;
> }
> double g(vec*x){
>   return (*x+*x)[3];
> }
> void h(vec*x, double a, double b, double c, double d){
>   vec y={a,b,c,d};
>   *x+=y;
> }
>
>
> I don't know if the patch (assuming it is ok) is suitable for stage 3, it
> may have to wait. It passes bootstrap+testsuite.
>
> I am still not quite sure why we even have a valid_gimple_rhs_p function
> (after which we usually give up if it says no) instead of gimplifying, so I
> may have missed a reason why CONSTRUCTOR or BIT_FIELD_REF shouldn't be ok.

Boostrapped and tested on ... ?

Ok if bootstrap / testing passes.

Thanks,
Richard.

>
> 2012-11-12  Marc Glisse  <marc.glisse@inria.fr>
>
>         * fold-const.c (fold_ternary_loc) [BIT_FIELD_REF]: Handle
>         CONSTRUCTOR with vector elements.
>         * tree-ssa-propagate.c (valid_gimple_rhs_p): Handle CONSTRUCTOR
>         and BIT_FIELD_REF.
>
> --
> Marc Glisse
> Index: gcc/fold-const.c
> ===================================================================
> --- gcc/fold-const.c    (revision 193429)
> +++ gcc/fold-const.c    (working copy)
> @@ -14054,65 +14054,75 @@ fold_ternary_loc (location_t loc, enum t
>           unsigned HOST_WIDE_INT n = tree_low_cst (arg1, 1);
>           unsigned HOST_WIDE_INT idx = tree_low_cst (op2, 1);
>
>           if (n != 0
>               && (idx % width) == 0
>               && (n % width) == 0
>               && ((idx + n) / width) <= TYPE_VECTOR_SUBPARTS (TREE_TYPE
> (arg0)))
>             {
>               idx = idx / width;
>               n = n / width;
> -             if (TREE_CODE (type) == VECTOR_TYPE)
> +
> +             if (TREE_CODE (arg0) == VECTOR_CST)
>                 {
> -                 if (TREE_CODE (arg0) == VECTOR_CST)
> -                   {
> -                     tree *vals = XALLOCAVEC (tree, n);
> -                     unsigned i;
> -                     for (i = 0; i < n; ++i)
> -                       vals[i] = VECTOR_CST_ELT (arg0, idx + i);
> -                     return build_vector (type, vals);
> -                   }
> -                 else
> -                   {
> -                     VEC(constructor_elt, gc) *vals;
> -                     unsigned i;
> -                     if (CONSTRUCTOR_NELTS (arg0) == 0)
> -                       return build_constructor (type, NULL);
> -                     if (TREE_CODE (TREE_TYPE (CONSTRUCTOR_ELT (arg0,
> -                                                                0)->value))
> -                         != VECTOR_TYPE)
> -                       {
> -                         vals = VEC_alloc (constructor_elt, gc, n);
> -                         for (i = 0;
> -                              i < n && idx + i < CONSTRUCTOR_NELTS (arg0);
> -                              ++i)
> -                           CONSTRUCTOR_APPEND_ELT (vals, NULL_TREE,
> -                                                   CONSTRUCTOR_ELT
> -                                                     (arg0, idx +
> i)->value);
> -                         return build_constructor (type, vals);
> -                       }
> -                   }
> +                 if (n == 1)
> +                   return VECTOR_CST_ELT (arg0, idx);
> +
> +                 tree *vals = XALLOCAVEC (tree, n);
> +                 for (unsigned i = 0; i < n; ++i)
> +                   vals[i] = VECTOR_CST_ELT (arg0, idx + i);
> +                 return build_vector (type, vals);
> +               }
> +
> +             /* Constructor elements can be subvectors.  */
> +             unsigned HOST_WIDE_INT k = 1;
> +             if (CONSTRUCTOR_NELTS (arg0) != 0)
> +               {
> +                 tree cons_elem = TREE_TYPE (CONSTRUCTOR_ELT (arg0,
> 0)->value);
> +                 if (TREE_CODE (cons_elem) == VECTOR_TYPE)
> +                   k = TYPE_VECTOR_SUBPARTS (cons_elem);
>                 }
> -             else if (n == 1)
> +
> +             /* We keep an exact subset of the constructor elements.  */
> +             if ((idx % k) == 0 && (n % k) == 0)
>                 {
> -                 if (TREE_CODE (arg0) == VECTOR_CST)
> -                   return VECTOR_CST_ELT (arg0, idx);
> -                 else if (CONSTRUCTOR_NELTS (arg0) == 0)
> -                   return build_zero_cst (type);
> -                 else if (TREE_CODE (TREE_TYPE (CONSTRUCTOR_ELT (arg0,
> -
> 0)->value))
> -                          != VECTOR_TYPE)
> +                 VEC(constructor_elt, gc) *vals;
> +                 if (CONSTRUCTOR_NELTS (arg0) == 0)
> +                   return build_constructor (type, NULL);
> +                 idx /= k;
> +                 n /= k;
> +                 if (n == 1)
>                     {
>                       if (idx < CONSTRUCTOR_NELTS (arg0))
>                         return CONSTRUCTOR_ELT (arg0, idx)->value;
>                       return build_zero_cst (type);
>                     }
> +                 vals = VEC_alloc (constructor_elt, gc, n);
> +                 for (unsigned i = 0;
> +                      i < n && idx + i < CONSTRUCTOR_NELTS (arg0);
> +                      ++i)
> +                   CONSTRUCTOR_APPEND_ELT (vals, NULL_TREE,
> +                                           CONSTRUCTOR_ELT
> +                                             (arg0, idx + i)->value);
> +                 return build_constructor (type, vals);
> +               }
> +             /* The bitfield references a single constructor element.  */
> +             else if (idx + n <= (idx / k + 1) * k)
> +               {
> +                 if (CONSTRUCTOR_NELTS (arg0) <= idx / k)
> +                   return build_zero_cst (type);
> +                 else if (n == k)
> +                   return CONSTRUCTOR_ELT (arg0, idx / k)->value;
> +                 else
> +                   return fold_build3_loc (loc, code, type,
> +                     CONSTRUCTOR_ELT (arg0, idx / k)->value, op1,
> +                     build_int_cst (TREE_TYPE (op2), (idx % k) * width));
>                 }
>             }
>         }
>
>        /* A bit-field-ref that referenced the full argument can be stripped.
> */
>        if (INTEGRAL_TYPE_P (TREE_TYPE (arg0))
>           && TYPE_PRECISION (TREE_TYPE (arg0)) == tree_low_cst (arg1, 1)
>           && integer_zerop (op2))
>         return fold_convert_loc (loc, type, arg0);
>
> Index: gcc/tree-ssa-propagate.c
> ===================================================================
> --- gcc/tree-ssa-propagate.c    (revision 193429)
> +++ gcc/tree-ssa-propagate.c    (working copy)
> @@ -610,24 +610,38 @@ valid_gimple_rhs_p (tree expr)
>               break;
>             }
>           return false;
>         }
>        break;
>
>      case tcc_vl_exp:
>        return false;
>
>      case tcc_exceptional:
> +      if (code == CONSTRUCTOR)
> +       {
> +         unsigned i;
> +         tree elt;
> +         FOR_EACH_CONSTRUCTOR_VALUE (CONSTRUCTOR_ELTS (expr), i, elt)
> +           if (!is_gimple_val (elt))
> +             return false;
> +         return true;
> +       }
>        if (code != SSA_NAME)
>          return false;
>        break;
>
> +    case tcc_reference:
> +      if (code == BIT_FIELD_REF)
> +       return is_gimple_val (TREE_OPERAND (expr, 0));
> +      return false;
> +
>      default:
>        return false;
>      }
>
>    return true;
>  }
>
>
>  /* Return true if EXPR is a CALL_EXPR suitable for representation
>     as a single GIMPLE_CALL statement.  If the arguments require
>
Marc Glisse - Nov. 27, 2012, 1:33 p.m.
On Tue, 27 Nov 2012, Richard Biener wrote:

> On Mon, Nov 12, 2012 at 2:31 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
>> Hello,
>>
>> this patch lets us fold a bit_field_ref of a constructor even when the
>> elements are vectors. Writing a testcase is not that convenient because of
>> the lack of a dead code elimination pass after forwprop4 (RTL doesn't always
>> remove everything either), but I see a clear difference on this test on
>> x86_64 without AVX.
>>
>> typedef double vec __attribute__((vector_size(4*sizeof(double))));
>> void f(vec*x){
>>   *x+=*x+*x;
>> }
>> double g(vec*x){
>>   return (*x+*x)[3];
>> }
>> void h(vec*x, double a, double b, double c, double d){
>>   vec y={a,b,c,d};
>>   *x+=y;
>> }
>>
>>
>> I don't know if the patch (assuming it is ok) is suitable for stage 3, it
>> may have to wait. It passes bootstrap+testsuite.

... on x86_64-linux-gnu, sorry for the lack of precision (default 
languages, Xeon E5520, graphite enabled).

>> I am still not quite sure why we even have a valid_gimple_rhs_p function
>> (after which we usually give up if it says no) instead of gimplifying, so I
>> may have missed a reason why CONSTRUCTOR or BIT_FIELD_REF shouldn't be ok.
>
> Boostrapped and tested on ... ?
>
> Ok if bootstrap / testing passes.

Thanks, I'll retest it to make sure it still works.

>> 2012-11-12  Marc Glisse  <marc.glisse@inria.fr>
>>
>>         * fold-const.c (fold_ternary_loc) [BIT_FIELD_REF]: Handle
>>         CONSTRUCTOR with vector elements.
>>         * tree-ssa-propagate.c (valid_gimple_rhs_p): Handle CONSTRUCTOR
>>         and BIT_FIELD_REF.

Patch

Index: gcc/fold-const.c

===================================================================
--- gcc/fold-const.c	(revision 193429)

+++ gcc/fold-const.c	(working copy)

@@ -14054,65 +14054,75 @@  fold_ternary_loc (location_t loc, enum t

 	  unsigned HOST_WIDE_INT n = tree_low_cst (arg1, 1);
 	  unsigned HOST_WIDE_INT idx = tree_low_cst (op2, 1);
 
 	  if (n != 0
 	      && (idx % width) == 0
 	      && (n % width) == 0
 	      && ((idx + n) / width) <= TYPE_VECTOR_SUBPARTS (TREE_TYPE (arg0)))
 	    {
 	      idx = idx / width;
 	      n = n / width;
-	      if (TREE_CODE (type) == VECTOR_TYPE)

+

+	      if (TREE_CODE (arg0) == VECTOR_CST)

 		{
-		  if (TREE_CODE (arg0) == VECTOR_CST)

-		    {

-		      tree *vals = XALLOCAVEC (tree, n);

-		      unsigned i;

-		      for (i = 0; i < n; ++i)

-			vals[i] = VECTOR_CST_ELT (arg0, idx + i);

-		      return build_vector (type, vals);

-		    }

-		  else

-		    {

-		      VEC(constructor_elt, gc) *vals;

-		      unsigned i;

-		      if (CONSTRUCTOR_NELTS (arg0) == 0)

-			return build_constructor (type, NULL);

-		      if (TREE_CODE (TREE_TYPE (CONSTRUCTOR_ELT (arg0,

-								 0)->value))

-			  != VECTOR_TYPE)

-			{

-			  vals = VEC_alloc (constructor_elt, gc, n);

-			  for (i = 0;

-			       i < n && idx + i < CONSTRUCTOR_NELTS (arg0);

-			       ++i)

-			    CONSTRUCTOR_APPEND_ELT (vals, NULL_TREE,

-						    CONSTRUCTOR_ELT

-						      (arg0, idx + i)->value);

-			  return build_constructor (type, vals);

-			}

-		    }

+		  if (n == 1)

+		    return VECTOR_CST_ELT (arg0, idx);

+

+		  tree *vals = XALLOCAVEC (tree, n);

+		  for (unsigned i = 0; i < n; ++i)

+		    vals[i] = VECTOR_CST_ELT (arg0, idx + i);

+		  return build_vector (type, vals);

+		}

+

+	      /* Constructor elements can be subvectors.  */

+	      unsigned HOST_WIDE_INT k = 1;

+	      if (CONSTRUCTOR_NELTS (arg0) != 0)

+		{

+		  tree cons_elem = TREE_TYPE (CONSTRUCTOR_ELT (arg0, 0)->value);

+		  if (TREE_CODE (cons_elem) == VECTOR_TYPE)

+		    k = TYPE_VECTOR_SUBPARTS (cons_elem);

 		}
-	      else if (n == 1)

+

+	      /* We keep an exact subset of the constructor elements.  */

+	      if ((idx % k) == 0 && (n % k) == 0)

 		{
-		  if (TREE_CODE (arg0) == VECTOR_CST)

-		    return VECTOR_CST_ELT (arg0, idx);

-		  else if (CONSTRUCTOR_NELTS (arg0) == 0)

-		    return build_zero_cst (type);

-		  else if (TREE_CODE (TREE_TYPE (CONSTRUCTOR_ELT (arg0,

-								  0)->value))

-			   != VECTOR_TYPE)

+		  VEC(constructor_elt, gc) *vals;

+		  if (CONSTRUCTOR_NELTS (arg0) == 0)

+		    return build_constructor (type, NULL);

+		  idx /= k;

+		  n /= k;

+		  if (n == 1)

 		    {
 		      if (idx < CONSTRUCTOR_NELTS (arg0))
 			return CONSTRUCTOR_ELT (arg0, idx)->value;
 		      return build_zero_cst (type);
 		    }
+		  vals = VEC_alloc (constructor_elt, gc, n);

+		  for (unsigned i = 0;

+		       i < n && idx + i < CONSTRUCTOR_NELTS (arg0);

+		       ++i)

+		    CONSTRUCTOR_APPEND_ELT (vals, NULL_TREE,

+					    CONSTRUCTOR_ELT

+					      (arg0, idx + i)->value);

+		  return build_constructor (type, vals);

+		}

+	      /* The bitfield references a single constructor element.  */

+	      else if (idx + n <= (idx / k + 1) * k)

+		{

+		  if (CONSTRUCTOR_NELTS (arg0) <= idx / k)

+		    return build_zero_cst (type);

+		  else if (n == k)

+		    return CONSTRUCTOR_ELT (arg0, idx / k)->value;

+		  else

+		    return fold_build3_loc (loc, code, type,

+		      CONSTRUCTOR_ELT (arg0, idx / k)->value, op1,

+		      build_int_cst (TREE_TYPE (op2), (idx % k) * width));

 		}
 	    }
 	}
 
       /* A bit-field-ref that referenced the full argument can be stripped.  */
       if (INTEGRAL_TYPE_P (TREE_TYPE (arg0))
 	  && TYPE_PRECISION (TREE_TYPE (arg0)) == tree_low_cst (arg1, 1)
 	  && integer_zerop (op2))
 	return fold_convert_loc (loc, type, arg0);
 
Index: gcc/tree-ssa-propagate.c

===================================================================
--- gcc/tree-ssa-propagate.c	(revision 193429)

+++ gcc/tree-ssa-propagate.c	(working copy)

@@ -610,24 +610,38 @@  valid_gimple_rhs_p (tree expr)

 	      break;
 	    }
 	  return false;
 	}
       break;
 
     case tcc_vl_exp:
       return false;
 
     case tcc_exceptional:
+      if (code == CONSTRUCTOR)

+	{

+	  unsigned i;

+	  tree elt;

+	  FOR_EACH_CONSTRUCTOR_VALUE (CONSTRUCTOR_ELTS (expr), i, elt)

+	    if (!is_gimple_val (elt))

+	      return false;

+	  return true;

+	}

       if (code != SSA_NAME)
         return false;
       break;
 
+    case tcc_reference:

+      if (code == BIT_FIELD_REF)

+	return is_gimple_val (TREE_OPERAND (expr, 0));

+      return false;

+

     default:
       return false;
     }
 
   return true;
 }
 
 
 /* Return true if EXPR is a CALL_EXPR suitable for representation
    as a single GIMPLE_CALL statement.  If the arguments require