Patchwork [C++] Fix __builtin_shuffle

login
register
mail settings
Submitter Marc Glisse
Date June 9, 2013, 11:09 a.m.
Message ID <alpine.DEB.2.02.1306091302380.27498@stedding.saclay.inria.fr>
Download mbox | patch
Permalink /patch/250026/
State New
Headers show

Comments

Marc Glisse - June 9, 2013, 11:09 a.m.
Hello,

when porting __builtin_shuffle from C to C++, I ignored all the C++ 
specificities and added some extra bugs. This should improve things a bit.

Bootstrap+testsuite on x86_64-linux-gnu.


2013-06-10  Marc Glisse  <marc.glisse@inria.fr>

 	PR c++/57509

gcc/c-family/
 	* c-common.h (c_build_vec_perm_expr): New complain argument.
 	* c-common.c (c_build_vec_perm_expr): Likewise.
 	Use save_expr also in C++.

gcc/cp/
 	* typeck.c (cp_build_vec_perm_expr): New function.
 	* cp-tree.h: Declare it.
 	* parser.c (cp_parser_postfix_expression): Call it.
 	* pt.c (tsubst_copy): Handle VEC_PERM_EXPR.
 	(tsubst_copy_and_build): Likewise.

gcc/testsuite/
 	* g++.dg/ext/pr57509.C: New file.
Marc Glisse - June 26, 2013, 2:20 p.m.
Ping http://gcc.gnu.org/ml/gcc-patches/2013-06/msg00470.html

On Sun, 9 Jun 2013, Marc Glisse wrote:

> Hello,
>
> when porting __builtin_shuffle from C to C++, I ignored all the C++ 
> specificities and added some extra bugs. This should improve things a bit.
>
> Bootstrap+testsuite on x86_64-linux-gnu.
>
>
> 2013-06-10  Marc Glisse  <marc.glisse@inria.fr>
>
> 	PR c++/57509
>
> gcc/c-family/
> 	* c-common.h (c_build_vec_perm_expr): New complain argument.
> 	* c-common.c (c_build_vec_perm_expr): Likewise.
> 	Use save_expr also in C++.
>
> gcc/cp/
> 	* typeck.c (cp_build_vec_perm_expr): New function.
> 	* cp-tree.h: Declare it.
> 	* parser.c (cp_parser_postfix_expression): Call it.
> 	* pt.c (tsubst_copy): Handle VEC_PERM_EXPR.
> 	(tsubst_copy_and_build): Likewise.
>
> gcc/testsuite/
> 	* g++.dg/ext/pr57509.C: New file.
Jason Merrill - June 26, 2013, 8:56 p.m.
On 06/09/2013 07:09 AM, Marc Glisse wrote:
> +      arg0 = build_non_dependent_expr (arg0);
> +      arg1 = build_non_dependent_expr (arg1);
> +      arg2 = build_non_dependent_expr (arg2);
> +    }
> +  return c_build_vec_perm_expr (loc, arg0, arg1, arg2, complain & tf_error);

This is wrong; the places in the compiler that currently use 
build_non_dependent_expr only use the result temporarily for determining 
the type of the expression, but then use the original arguments for 
building the return value.

Jason

Patch

Index: c-family/c-common.c

===================================================================
--- c-family/c-common.c	(revision 199867)

+++ c-family/c-common.c	(working copy)

@@ -2253,95 +2253,103 @@  vector_types_convertible_p (const_tree t

    and have vector types, V0 has the same type as V1, and the number of
    elements of V0, V1, MASK is the same.
 
    In case V1 is a NULL_TREE it is assumed that __builtin_shuffle was
    called with two arguments.  In this case implementation passes the
    first argument twice in order to share the same tree code.  This fact
    could enable the mask-values being twice the vector length.  This is
    an implementation accident and this semantics is not guaranteed to
    the user.  */
 tree
-c_build_vec_perm_expr (location_t loc, tree v0, tree v1, tree mask)

+c_build_vec_perm_expr (location_t loc, tree v0, tree v1, tree mask,

+		       bool complain)

 {
   tree ret;
   bool wrap = true;
   bool maybe_const = false;
   bool two_arguments = false;
 
   if (v1 == NULL_TREE)
     {
       two_arguments = true;
       v1 = v0;
     }
 
   if (v0 == error_mark_node || v1 == error_mark_node
       || mask == error_mark_node)
     return error_mark_node;
 
   if (TREE_CODE (TREE_TYPE (mask)) != VECTOR_TYPE
       || TREE_CODE (TREE_TYPE (TREE_TYPE (mask))) != INTEGER_TYPE)
     {
-      error_at (loc, "__builtin_shuffle last argument must "

-		     "be an integer vector");

+      if (complain)

+	error_at (loc, "__builtin_shuffle last argument must "

+		       "be an integer vector");

       return error_mark_node;
     }
 
   if (TREE_CODE (TREE_TYPE (v0)) != VECTOR_TYPE
       || TREE_CODE (TREE_TYPE (v1)) != VECTOR_TYPE)
     {
-      error_at (loc, "__builtin_shuffle arguments must be vectors");

+      if (complain)

+	error_at (loc, "__builtin_shuffle arguments must be vectors");

       return error_mark_node;
     }
 
   if (TYPE_MAIN_VARIANT (TREE_TYPE (v0)) != TYPE_MAIN_VARIANT (TREE_TYPE (v1)))
     {
-      error_at (loc, "__builtin_shuffle argument vectors must be of "

-		     "the same type");

+      if (complain)

+	error_at (loc, "__builtin_shuffle argument vectors must be of "

+		       "the same type");

       return error_mark_node;
     }
 
   if (TYPE_VECTOR_SUBPARTS (TREE_TYPE (v0))
       != TYPE_VECTOR_SUBPARTS (TREE_TYPE (mask))
       && TYPE_VECTOR_SUBPARTS (TREE_TYPE (v1))
 	 != TYPE_VECTOR_SUBPARTS (TREE_TYPE (mask)))
     {
-      error_at (loc, "__builtin_shuffle number of elements of the "

-		     "argument vector(s) and the mask vector should "

-		     "be the same");

+      if (complain)

+	error_at (loc, "__builtin_shuffle number of elements of the "

+		       "argument vector(s) and the mask vector should "

+		       "be the same");

       return error_mark_node;
     }
 
   if (GET_MODE_BITSIZE (TYPE_MODE (TREE_TYPE (TREE_TYPE (v0))))
       != GET_MODE_BITSIZE (TYPE_MODE (TREE_TYPE (TREE_TYPE (mask)))))
     {
-      error_at (loc, "__builtin_shuffle argument vector(s) inner type "

-		     "must have the same size as inner type of the mask");

+      if (complain)

+	error_at (loc, "__builtin_shuffle argument vector(s) inner type "

+		       "must have the same size as inner type of the mask");

       return error_mark_node;
     }
 
   if (!c_dialect_cxx ())
     {
       /* Avoid C_MAYBE_CONST_EXPRs inside VEC_PERM_EXPR.  */
       v0 = c_fully_fold (v0, false, &maybe_const);
       wrap &= maybe_const;
 
       if (two_arguments)
         v1 = v0 = save_expr (v0);
       else
         {
           v1 = c_fully_fold (v1, false, &maybe_const);
           wrap &= maybe_const;
         }
 
       mask = c_fully_fold (mask, false, &maybe_const);
       wrap &= maybe_const;
     }
+  else if (two_arguments)

+    v1 = v0 = save_expr (v0);

 
   ret = build3_loc (loc, VEC_PERM_EXPR, TREE_TYPE (v0), v0, v1, mask);
 
   if (!c_dialect_cxx () && !wrap)
     ret = c_wrap_maybe_const (ret, true);
 
   return ret;
 }
 
 /* Like tree.c:get_narrower, but retain conversion from C++0x scoped enum
Index: c-family/c-common.h

===================================================================
--- c-family/c-common.h	(revision 199867)

+++ c-family/c-common.h	(working copy)

@@ -904,21 +904,21 @@  extern tree resolve_overloaded_builtin (

 extern tree finish_label_address_expr (tree, location_t);
 
 /* Same function prototype, but the C and C++ front ends have
    different implementations.  Used in c-common.c.  */
 extern tree lookup_label (tree);
 extern tree lookup_name (tree);
 extern bool lvalue_p (const_tree);
 
 extern bool vector_targets_convertible_p (const_tree t1, const_tree t2);
 extern bool vector_types_convertible_p (const_tree t1, const_tree t2, bool emit_lax_note);
-extern tree c_build_vec_perm_expr (location_t, tree, tree, tree);

+extern tree c_build_vec_perm_expr (location_t, tree, tree, tree, bool = true);

 
 extern rtx c_expand_expr (tree, rtx, enum machine_mode, int, rtx *);
 
 extern void init_c_lex (void);
 
 extern void c_cpp_builtins (cpp_reader *);
 extern void c_cpp_builtins_optimize_pragma (cpp_reader *, tree, tree);
 extern bool c_cpp_error (cpp_reader *, int, int, location_t, unsigned int,
 			 const char *, va_list *)
      ATTRIBUTE_GCC_DIAG(6,0);
Index: testsuite/g++.dg/ext/pr57509.C

===================================================================
--- testsuite/g++.dg/ext/pr57509.C	(revision 0)

+++ testsuite/g++.dg/ext/pr57509.C	(revision 0)

@@ -0,0 +1,16 @@ 

+/* { dg-do compile } */

+/* { dg-options "-std=c++11" } */

+

+template <bool> struct enable_if {};

+template <> struct enable_if<true> {typedef void type;};

+template <class T> void f (T& v) { v = __builtin_shuffle (v, v); }

+template <class T> void g (T) {}

+template <class T> auto g (T x) -> typename enable_if<sizeof(__builtin_shuffle(x,x))!=2>::type {}

+typedef int v4i __attribute__((vector_size(4*sizeof(int))));

+typedef float v4f __attribute__((vector_size(4*sizeof(float))));

+int main(){

+  v4i a = {1,2,3,0};

+  f(a);

+  v4f b = {1,2,3,0};

+  g(b);

+}


Property changes on: testsuite/g++.dg/ext/pr57509.C
___________________________________________________________________
Added: svn:eol-style
   + native
Added: svn:keywords
   + Author Date Id Revision URL

Index: cp/typeck.c

===================================================================
--- cp/typeck.c	(revision 199867)

+++ cp/typeck.c	(working copy)

@@ -4837,20 +4837,40 @@  cp_build_binary_op (location_t location,

   if (final_type != 0)
     result = cp_convert (final_type, result, complain);
 
   if (TREE_OVERFLOW_P (result) 
       && !TREE_OVERFLOW_P (op0) 
       && !TREE_OVERFLOW_P (op1))
     overflow_warning (location, result);
 
   return result;
 }
+

+/* Build a VEC_PERM_EXPR.

+   This is a simple wrapper for c_build_vec_perm_expr.  */

+tree

+cp_build_vec_perm_expr (location_t loc,

+			tree arg0, tree arg1, tree arg2,

+			tsubst_flags_t complain)

+{

+  if (processing_template_decl)

+    {

+      if (type_dependent_expression_p (arg0)

+	  || type_dependent_expression_p (arg1)

+	  || type_dependent_expression_p (arg2))

+	return build_min_nt_loc (loc, VEC_PERM_EXPR, arg0, arg1, arg2);

+      arg0 = build_non_dependent_expr (arg0);

+      arg1 = build_non_dependent_expr (arg1);

+      arg2 = build_non_dependent_expr (arg2);

+    }

+  return c_build_vec_perm_expr (loc, arg0, arg1, arg2, complain & tf_error);

+}

 
 /* Return a tree for the sum or difference (RESULTCODE says which)
    of pointer PTROP and integer INTOP.  */
 
 static tree
 cp_pointer_int_sum (enum tree_code resultcode, tree ptrop, tree intop)
 {
   tree res_type = TREE_TYPE (ptrop);
 
   /* pointer_int_sum() uses size_in_bytes() on the TREE_TYPE(res_type)
Index: cp/cp-tree.h

===================================================================
--- cp/cp-tree.h	(revision 199867)

+++ cp/cp-tree.h	(working copy)

@@ -6032,20 +6032,23 @@  extern tree type_after_usual_arithmetic_

 extern tree common_pointer_type                 (tree, tree);
 extern tree composite_pointer_type		(tree, tree, tree, tree,
 						 composite_pointer_operation, 
 						 tsubst_flags_t);
 extern tree merge_types				(tree, tree);
 extern tree strip_array_domain			(tree);
 extern tree check_return_expr			(tree, bool *);
 extern tree cp_build_binary_op                  (location_t,
 						 enum tree_code, tree, tree,
 						 tsubst_flags_t);
+extern tree cp_build_vec_perm_expr              (location_t,

+						 tree, tree, tree,

+						 tsubst_flags_t);

 #define cxx_sizeof(T)  cxx_sizeof_or_alignof_type (T, SIZEOF_EXPR, true)
 extern tree build_simple_component_ref		(tree, tree);
 extern tree build_ptrmemfunc_access_expr	(tree, tree);
 extern tree build_address			(tree);
 extern tree build_typed_address			(tree, tree);
 extern tree build_nop				(tree, tree);
 extern tree non_reference			(tree);
 extern tree lookup_anon_field			(tree, tree);
 extern bool invalid_nonstatic_memfn_p		(tree, tsubst_flags_t);
 extern tree convert_member_func_to_ptr		(tree, tree, tsubst_flags_t);
Index: cp/pt.c

===================================================================
--- cp/pt.c	(revision 199867)

+++ cp/pt.c	(working copy)

@@ -12444,20 +12444,21 @@  tsubst_copy (tree t, tree args, tsubst_f

 	int i;
 	for (i = 0; i < n; i++)
 	  TREE_OPERAND (t, i) = tsubst_copy (TREE_OPERAND (t, i), args,
 					     complain, in_decl);
 	return result;
       }
 
     case COND_EXPR:
     case MODOP_EXPR:
     case PSEUDO_DTOR_EXPR:
+    case VEC_PERM_EXPR:

       {
 	r = build_nt
 	  (code, tsubst_copy (TREE_OPERAND (t, 0), args, complain, in_decl),
 	   tsubst_copy (TREE_OPERAND (t, 1), args, complain, in_decl),
 	   tsubst_copy (TREE_OPERAND (t, 2), args, complain, in_decl));
 	TREE_NO_WARNING (r) = TREE_NO_WARNING (t);
 	return r;
       }
 
     case NEW_EXPR:
@@ -14594,20 +14595,27 @@  tsubst_copy_and_build (tree t,

 	RETURN (r);
       }
 
     case TRANSACTION_EXPR:
       RETURN (tsubst_expr(t, args, complain, in_decl,
 	     integral_constant_expression_p));
 
     case PAREN_EXPR:
       RETURN (finish_parenthesized_expr (RECUR (TREE_OPERAND (t, 0))));
 
+    case VEC_PERM_EXPR:

+      RETURN (cp_build_vec_perm_expr (input_location,

+		RECUR (TREE_OPERAND (t, 0)),

+		RECUR (TREE_OPERAND (t, 1)),

+		RECUR (TREE_OPERAND (t, 2)),

+		complain));

+

     default:
       /* Handle Objective-C++ constructs, if appropriate.  */
       {
 	tree subst
 	  = objcp_tsubst_copy_and_build (t, args, complain,
 					 in_decl, /*function_p=*/false);
 	if (subst)
 	  RETURN (subst);
       }
       RETURN (tsubst_copy (t, args, complain, in_decl));
Index: cp/parser.c

===================================================================
--- cp/parser.c	(revision 199867)

+++ cp/parser.c	(working copy)

@@ -5678,23 +5678,25 @@  cp_parser_postfix_expression (cp_parser

 	vec = cp_parser_parenthesized_expression_list (parser, non_attr,
 		    /*cast_p=*/false, /*allow_expansion_p=*/true,
 		    /*non_constant_p=*/NULL);
 	if (vec == NULL)
 	  return error_mark_node;
 
 	FOR_EACH_VEC_ELT (*vec, i, p)
 	  mark_exp_read (p);
 
 	if (vec->length () == 2)
-	  return c_build_vec_perm_expr (loc, (*vec)[0], NULL_TREE, (*vec)[1]); 

+	  return cp_build_vec_perm_expr (loc, (*vec)[0], NULL_TREE, (*vec)[1],

+					 tf_warning_or_error);

 	else if (vec->length () == 3)
-	  return c_build_vec_perm_expr (loc, (*vec)[0], (*vec)[1], (*vec)[2]);

+	  return cp_build_vec_perm_expr (loc, (*vec)[0], (*vec)[1], (*vec)[2],

+					 tf_warning_or_error);

 	else
 	{
 	  error_at (loc, "wrong number of arguments to "
 	      "%<__builtin_shuffle%>");
 	  return error_mark_node;
 	}
 	break;
       }
 
     default: