diff mbox series

[1/8] rs6000: More factoring of overload processing

Message ID 9ee506e947ec49973f757ea4a967574ded4ed2b0.1643390744.git.wschmidt@linux.ibm.com
State New
Headers
Series rs6000: Built-in function cleanups and bug fixes |

Commit Message

Bill Schmidt Jan. 28, 2022, 5:50 p.m. UTC 
  This patch continues the refactoring started with r12-6014.  I had previously
noted that the resolve_vec* routines can be further simplified by processing
the argument list earlier, so that all routines can use the arrays of arguments
and types.  I found that this was useful for some of the routines, but not for
all of them.

For several of the special-cased overloads, we don't specify all of the
possible type combinations in rs6000-overload.def, because the types don't
matter for the expansion we do.  For these, we can't use generic error message
handling when the number of arguments is incorrect, because the result is
misleading error messages that indicate argument types are wrong.

So this patch goes halfway and improves the factoring on the remaining special
cases, but leaves vec_splats, vec_promote, vec_extract, vec_insert, and
vec_step alone.

Bootstrapped and tested on powerpc64le-linux-gnu with no regressions.
Is this okay for trunk?

Thanks,
Bill


2022-01-18  Bill Schmidt  <wschmidt@linux.ibm.com>

gcc/
	* config/rs6000/rs6000-c.cc (resolve_vec_mul): Accept args and types
	parameters instead of arglist and nargs.  Simplify accordingly.  Remove
	unnecessary test for argument count mismatch.
	(resolve_vec_cmpne): Likewise.
	(resolve_vec_adde_sube): Likewise.
	(resolve_vec_addec_subec): Likewise.
	(altivec_resolve_overloaded_builtin): Move overload special handling
	after the gathering of arguments into args[] and types[] and the test
	for correct number of arguments.  Don't perform the test for correct
	number of arguments for certain special cases.  Call the other special
	cases with args and types instead of arglist and nargs.
---
 gcc/config/rs6000/rs6000-c.cc | 304 ++++++++++++++--------------------
 1 file changed, 127 insertions(+), 177 deletions(-)

Comments

Segher Boessenkool Jan. 28, 2022, 7:11 p.m. UTC | #1 
On Fri, Jan 28, 2022 at 11:50:19AM -0600, Bill Schmidt wrote:
> This patch continues the refactoring started with r12-6014.

ab3f5b71dc6e

> +     and the generic code will issue the appropriate error message.  Skip
> +     this test for functions where we don't fully describe all the possible
> +     overload signatures in rs6000-overload.def (because they aren't relevant
> +     to the expansion here).  If we don't, we get confusing error messages.  */
> +  if (fcode != RS6000_OVLD_VEC_PROMOTE
> +      && fcode != RS6000_OVLD_VEC_SPLATS
> +      && fcode != RS6000_OVLD_VEC_EXTRACT
> +      && fcode != RS6000_OVLD_VEC_INSERT
> +      && fcode != RS6000_OVLD_VEC_STEP
> +      && (!VOID_TYPE_P (TREE_VALUE (fnargs)) || n < nargs))
>      return NULL;

Can you expand a bit on this, give an example for example?  It is very
hard to understand this code, the way it depends on code following many
lines later.

> +    default:
> +      ;

Don't.

I like this better than a BS break statement, but it is just as stupid.

If you need this, you don't want a switch statement, but some number of
if statements.  You cannot use a switch as a shorthand for this because
we have a silly warning and -Werror for this use.

You probably get easier to understand code that way, too, you can get
rid of the above (just do some early returns), etc.


Segher
Li, Pan2 via Gcc-patches Jan. 28, 2022, 9:19 p.m. UTC | #2 
On 1/28/22 1:11 PM, Segher Boessenkool wrote:
> On Fri, Jan 28, 2022 at 11:50:19AM -0600, Bill Schmidt wrote:
>> This patch continues the refactoring started with r12-6014.
> ab3f5b71dc6e
>
>> +     and the generic code will issue the appropriate error message.  Skip
>> +     this test for functions where we don't fully describe all the possible
>> +     overload signatures in rs6000-overload.def (because they aren't relevant
>> +     to the expansion here).  If we don't, we get confusing error messages.  */
>> +  if (fcode != RS6000_OVLD_VEC_PROMOTE
>> +      && fcode != RS6000_OVLD_VEC_SPLATS
>> +      && fcode != RS6000_OVLD_VEC_EXTRACT
>> +      && fcode != RS6000_OVLD_VEC_INSERT
>> +      && fcode != RS6000_OVLD_VEC_STEP
>> +      && (!VOID_TYPE_P (TREE_VALUE (fnargs)) || n < nargs))
>>      return NULL;
> Can you expand a bit on this, give an example for example?  It is very
> hard to understand this code, the way it depends on code following many
> lines later.

Sure, sorry.

This check gives up if the number of arguments doesn't match the prototype.
It gives a fairly generic error message.  That part of it has always been
in here.

Now, I moved this check forward relative to the big switch statement on
fcode, because there are redundant checks for the number of arguments
in each of the resolve_vec_* helper functions.  This allowed me to simplify
those a bit.

Now, it turns out that this doesn't work so well for functions that aren't
fully described in rs6000-overload.def.  For example, for vec_splats we
have:

; There are no actual builtins for vec_splats.  There is special handling for
; this in altivec_resolve_overloaded_builtin in rs6000-c.cc, where the call
; is replaced by a constructor.  The single overload here causes
; __builtin_vec_splats to be registered with the front end so that can happen.
[VEC_SPLATS, vec_splats, __builtin_vec_splats]
  vsi __builtin_vec_splats (vsi);
    ABS_V4SI SPLATS_FAKERY

So even though __builtin_vec_splats accepts all vector types, the
infrastructure cheats and just records one prototype.  We end up getting
an error message that refers to this specific prototype even when we are
handling a different argument type.  That is completely confusing to the
user.  So I felt I was starting to get too deep for a simple refactoring
patch, and gave up on early number-of-arguments checking for the special
cases that use the _FAKERY technique.

That's probably still not clear, but maybe clearer?

>
>> +    default:
>> +      ;
> Don't.
>
> I like this better than a BS break statement, but it is just as stupid.
>
> If you need this, you don't want a switch statement, but some number of
> if statements.  You cannot use a switch as a shorthand for this because
> we have a silly warning and -Werror for this use.
>
> You probably get easier to understand code that way, too, you can get
> rid of the above (just do some early returns), etc.

If I understand correctly, you'd like me to resubmit this in if-then-else
form.  That's fine, just want to be sure that's what you want.

Thanks for the review!
Bill

>
>
> Segher
Segher Boessenkool Jan. 28, 2022, 11:09 p.m. UTC | #3 
On Fri, Jan 28, 2022 at 03:19:48PM -0600, Bill Schmidt wrote:
> On 1/28/22 1:11 PM, Segher Boessenkool wrote:
> > On Fri, Jan 28, 2022 at 11:50:19AM -0600, Bill Schmidt wrote:
> >> +     and the generic code will issue the appropriate error message.  Skip
> >> +     this test for functions where we don't fully describe all the possible
> >> +     overload signatures in rs6000-overload.def (because they aren't relevant
> >> +     to the expansion here).  If we don't, we get confusing error messages.  */
> >> +  if (fcode != RS6000_OVLD_VEC_PROMOTE
> >> +      && fcode != RS6000_OVLD_VEC_SPLATS
> >> +      && fcode != RS6000_OVLD_VEC_EXTRACT
> >> +      && fcode != RS6000_OVLD_VEC_INSERT
> >> +      && fcode != RS6000_OVLD_VEC_STEP
> >> +      && (!VOID_TYPE_P (TREE_VALUE (fnargs)) || n < nargs))
> >>      return NULL;
> > Can you expand a bit on this, give an example for example?  It is very
> > hard to understand this code, the way it depends on code following many
> > lines later.
> 
> Sure, sorry.
> 
> This check gives up if the number of arguments doesn't match the prototype.
> It gives a fairly generic error message.  That part of it has always been
> in here.
> 
> Now, I moved this check forward relative to the big switch statement on
> fcode, because there are redundant checks for the number of arguments
> in each of the resolve_vec_* helper functions.  This allowed me to simplify
> those a bit.
> 
> Now, it turns out that this doesn't work so well for functions that aren't
> fully described in rs6000-overload.def.  For example, for vec_splats we
> have:
> 
> ; There are no actual builtins for vec_splats.  There is special handling for
> ; this in altivec_resolve_overloaded_builtin in rs6000-c.cc, where the call
> ; is replaced by a constructor.  The single overload here causes
> ; __builtin_vec_splats to be registered with the front end so that can happen.
> [VEC_SPLATS, vec_splats, __builtin_vec_splats]
>   vsi __builtin_vec_splats (vsi);
>     ABS_V4SI SPLATS_FAKERY
> 
> So even though __builtin_vec_splats accepts all vector types, the
> infrastructure cheats and just records one prototype.  We end up getting
> an error message that refers to this specific prototype even when we are
> handling a different argument type.  That is completely confusing to the
> user.  So I felt I was starting to get too deep for a simple refactoring
> patch, and gave up on early number-of-arguments checking for the special
> cases that use the _FAKERY technique.
> 
> That's probably still not clear, but maybe clearer?

Much better, thanks!

So put a comment before the code handling the arg checking for
vec_splats etc. saying just that?  Or the much condensed form "these
codes should be handled separately because <bla>" :-)  (And the larger
explanation in the commit message -- there you can talk about the old
code / old situation as well :-) )

> >> +    default:
> >> +      ;
> > Don't.
> >
> > I like this better than a BS break statement, but it is just as stupid.
> >
> > If you need this, you don't want a switch statement, but some number of
> > if statements.  You cannot use a switch as a shorthand for this because
> > we have a silly warning and -Werror for this use.
> >
> > You probably get easier to understand code that way, too, you can get
> > rid of the above (just do some early returns), etc.
> 
> If I understand correctly, you'd like me to resubmit this in if-then-else
> form.  That's fine, just want to be sure that's what you want.

Yes please.  This is new code, so let's please keep it as readable as
possible.  Since you need to redo some of it anyway as well...


Segher
diff mbox series

Patch

diff --git a/gcc/config/rs6000/rs6000-c.cc b/gcc/config/rs6000/rs6000-c.cc
index 145421ab8f2..35c1383f059 100644
--- a/gcc/config/rs6000/rs6000-c.cc
+++ b/gcc/config/rs6000/rs6000-c.cc
@@ -939,37 +939,25 @@  altivec_build_resolved_builtin (tree *args, int n, tree fntype, tree ret_type,
 enum resolution { unresolved, resolved, resolved_bad };
 
 /* Resolve an overloaded vec_mul call and return a tree expression for the
-   resolved call if successful.  NARGS is the number of arguments to the call.
-   ARGLIST contains the arguments.  RES must be set to indicate the status of
+   resolved call if successful.  ARGS contains the arguments to the call.
+   TYPES contains their types.  RES must be set to indicate the status of
    the resolution attempt.  LOC contains statement location information.  */
 
 static tree
-resolve_vec_mul (resolution *res, vec<tree, va_gc> *arglist, unsigned nargs,
-		 location_t loc)
+resolve_vec_mul (resolution *res, tree *args, tree *types, location_t loc)
 {
   /* vec_mul needs to be special cased because there are no instructions for it
      for the {un}signed char, {un}signed short, and {un}signed int types.  */
-  if (nargs != 2)
-    {
-      error ("builtin %qs only accepts 2 arguments", "vec_mul");
-      *res = resolved;
-      return error_mark_node;
-    }
-
-  tree arg0 = (*arglist)[0];
-  tree arg0_type = TREE_TYPE (arg0);
-  tree arg1 = (*arglist)[1];
-  tree arg1_type = TREE_TYPE (arg1);
 
   /* Both arguments must be vectors and the types must be compatible.  */
-  if (TREE_CODE (arg0_type) != VECTOR_TYPE
-      || !lang_hooks.types_compatible_p (arg0_type, arg1_type))
+  if (TREE_CODE (types[0]) != VECTOR_TYPE
+      || !lang_hooks.types_compatible_p (types[0], types[1]))
     {
       *res = resolved_bad;
       return error_mark_node;
     }
 
-  switch (TYPE_MODE (TREE_TYPE (arg0_type)))
+  switch (TYPE_MODE (TREE_TYPE (types[0])))
     {
     case E_QImode:
     case E_HImode:
@@ -978,21 +966,21 @@  resolve_vec_mul (resolution *res, vec<tree, va_gc> *arglist, unsigned nargs,
     case E_TImode:
       /* For scalar types just use a multiply expression.  */
       *res = resolved;
-      return fold_build2_loc (loc, MULT_EXPR, TREE_TYPE (arg0), arg0,
-			      fold_convert (TREE_TYPE (arg0), arg1));
+      return fold_build2_loc (loc, MULT_EXPR, types[0], args[0],
+			      fold_convert (types[0], args[1]));
     case E_SFmode:
       {
 	/* For floats use the xvmulsp instruction directly.  */
 	*res = resolved;
 	tree call = rs6000_builtin_decls[RS6000_BIF_XVMULSP];
-	return build_call_expr (call, 2, arg0, arg1);
+	return build_call_expr (call, 2, args[0], args[1]);
       }
     case E_DFmode:
       {
 	/* For doubles use the xvmuldp instruction directly.  */
 	*res = resolved;
 	tree call = rs6000_builtin_decls[RS6000_BIF_XVMULDP];
-	return build_call_expr (call, 2, arg0, arg1);
+	return build_call_expr (call, 2, args[0], args[1]);
       }
     /* Other types are errors.  */
     default:
@@ -1002,37 +990,25 @@  resolve_vec_mul (resolution *res, vec<tree, va_gc> *arglist, unsigned nargs,
 }
 
 /* Resolve an overloaded vec_cmpne call and return a tree expression for the
-   resolved call if successful.  NARGS is the number of arguments to the call.
-   ARGLIST contains the arguments.  RES must be set to indicate the status of
+   resolved call if successful.  ARGS contains the arguments to the call.
+   TYPES contains their types.  RES must be set to indicate the status of
    the resolution attempt.  LOC contains statement location information.  */
 
 static tree
-resolve_vec_cmpne (resolution *res, vec<tree, va_gc> *arglist, unsigned nargs,
-		   location_t loc)
+resolve_vec_cmpne (resolution *res, tree *args, tree *types, location_t loc)
 {
   /* vec_cmpne needs to be special cased because there are no instructions
      for it (prior to power 9).  */
-  if (nargs != 2)
-    {
-      error ("builtin %qs only accepts 2 arguments", "vec_cmpne");
-      *res = resolved;
-      return error_mark_node;
-    }
-
-  tree arg0 = (*arglist)[0];
-  tree arg0_type = TREE_TYPE (arg0);
-  tree arg1 = (*arglist)[1];
-  tree arg1_type = TREE_TYPE (arg1);
 
   /* Both arguments must be vectors and the types must be compatible.  */
-  if (TREE_CODE (arg0_type) != VECTOR_TYPE
-      || !lang_hooks.types_compatible_p (arg0_type, arg1_type))
+  if (TREE_CODE (types[0]) != VECTOR_TYPE
+      || !lang_hooks.types_compatible_p (types[0], types[1]))
     {
       *res = resolved_bad;
       return error_mark_node;
     }
 
-  machine_mode arg0_elt_mode = TYPE_MODE (TREE_TYPE (arg0_type));
+  machine_mode arg0_elt_mode = TYPE_MODE (TREE_TYPE (types[0]));
 
   /* Power9 instructions provide the most efficient implementation of
      ALTIVEC_BUILTIN_VEC_CMPNE if the mode is not DImode or TImode
@@ -1060,8 +1036,8 @@  resolve_vec_cmpne (resolution *res, vec<tree, va_gc> *arglist, unsigned nargs,
 	    /* call = vec_cmpeq (va, vb)
 	       result = vec_nor (call, call).  */
 	    vec<tree, va_gc> *params = make_tree_vector ();
-	    vec_safe_push (params, arg0);
-	    vec_safe_push (params, arg1);
+	    vec_safe_push (params, args[0]);
+	    vec_safe_push (params, args[1]);
 	    tree decl = rs6000_builtin_decls[RS6000_OVLD_VEC_CMPEQ];
 	    tree call = altivec_resolve_overloaded_builtin (loc, decl, params);
 	    /* Use save_expr to ensure that operands used more than once
@@ -1088,46 +1064,30 @@  resolve_vec_cmpne (resolution *res, vec<tree, va_gc> *arglist, unsigned nargs,
   return error_mark_node;
 }
 
-/* Resolve an overloaded vec_adde or vec_sube call and return a tree
-   expression for the resolved call if successful.  NARGS is the number of
-   arguments to the call.  ARGLIST contains the arguments.  RES must be set
-   to indicate the status of the resolution attempt.  LOC contains statement
-   location information.  */
+/* Resolve an overloaded vec_adde or vec_sube call and return a tree expression
+   for the resolved call if successful.  ARGS contains the arguments to the
+   call.  TYPES contains their arguments.  RES must be set to indicate the
+   status of the resolution attempt.  LOC contains statement location
+   information.  */
 
 static tree
 resolve_vec_adde_sube (resolution *res, rs6000_gen_builtins fcode,
-		       vec<tree, va_gc> *arglist, unsigned nargs,
-		       location_t loc)
+		       tree *args, tree *types, location_t loc)
 {
   /* vec_adde needs to be special cased because there is no instruction
      for the {un}signed int version.  */
-  if (nargs != 3)
-    {
-      const char *name;
-      name = fcode == RS6000_OVLD_VEC_ADDE ? "vec_adde" : "vec_sube";
-      error ("builtin %qs only accepts 3 arguments", name);
-      *res = resolved;
-      return error_mark_node;
-    }
-
-  tree arg0 = (*arglist)[0];
-  tree arg0_type = TREE_TYPE (arg0);
-  tree arg1 = (*arglist)[1];
-  tree arg1_type = TREE_TYPE (arg1);
-  tree arg2 = (*arglist)[2];
-  tree arg2_type = TREE_TYPE (arg2);
 
   /* All 3 arguments must be vectors of (signed or unsigned) (int or
      __int128) and the types must be compatible.  */
-  if (TREE_CODE (arg0_type) != VECTOR_TYPE
-      || !lang_hooks.types_compatible_p (arg0_type, arg1_type)
-      || !lang_hooks.types_compatible_p (arg1_type, arg2_type))
+  if (TREE_CODE (types[0]) != VECTOR_TYPE
+      || !lang_hooks.types_compatible_p (types[0], types[1])
+      || !lang_hooks.types_compatible_p (types[1], types[2]))
     {
       *res = resolved_bad;
       return error_mark_node;
     }
 
-  switch (TYPE_MODE (TREE_TYPE (arg0_type)))
+  switch (TYPE_MODE (TREE_TYPE (types[0])))
     {
       /* For {un}signed ints,
 	 vec_adde (va, vb, carryv) == vec_add (vec_add (va, vb),
@@ -1137,8 +1097,8 @@  resolve_vec_adde_sube (resolution *res, rs6000_gen_builtins fcode,
     case E_SImode:
       {
 	vec<tree, va_gc> *params = make_tree_vector ();
-	vec_safe_push (params, arg0);
-	vec_safe_push (params, arg1);
+	vec_safe_push (params, args[0]);
+	vec_safe_push (params, args[1]);
 
 	tree add_sub_builtin;
 	if (fcode == RS6000_OVLD_VEC_ADDE)
@@ -1148,10 +1108,10 @@  resolve_vec_adde_sube (resolution *res, rs6000_gen_builtins fcode,
 
 	tree call = altivec_resolve_overloaded_builtin (loc, add_sub_builtin,
 							params);
-	tree const1 = build_int_cstu (TREE_TYPE (arg0_type), 1);
-	tree ones_vector = build_vector_from_val (arg0_type, const1);
-	tree and_expr = fold_build2_loc (loc, BIT_AND_EXPR, arg0_type,
-					 arg2, ones_vector);
+	tree const1 = build_int_cstu (TREE_TYPE (types[0]), 1);
+	tree ones_vector = build_vector_from_val (types[0], const1);
+	tree and_expr = fold_build2_loc (loc, BIT_AND_EXPR, types[0],
+					 args[2], ones_vector);
 	params = make_tree_vector ();
 	vec_safe_push (params, call);
 	vec_safe_push (params, and_expr);
@@ -1175,45 +1135,29 @@  resolve_vec_adde_sube (resolution *res, rs6000_gen_builtins fcode,
 }
 
 /* Resolve an overloaded vec_addec or vec_subec call and return a tree
-   expression for the resolved call if successful.  NARGS is the number of
-   arguments to the call.  ARGLIST contains the arguments.  RES must be set
-   to indicate the status of the resolution attempt.  LOC contains statement
-   location information.  */
+   expression for the resolved call if successful.  ARGS contains the arguments
+   to the call.  TYPES contains their types.  RES must be set to indicate the
+   status of the resolution attempt.  LOC contains statement location
+   information.  */
 
 static tree
 resolve_vec_addec_subec (resolution *res, rs6000_gen_builtins fcode,
-			 vec<tree, va_gc> *arglist, unsigned nargs,
-			 location_t loc)
+			 tree *args, tree *types, location_t loc)
 {
   /* vec_addec and vec_subec needs to be special cased because there is
      no instruction for the (un)signed int version.  */
-  if (nargs != 3)
-    {
-      const char *name;
-      name = fcode == RS6000_OVLD_VEC_ADDEC ? "vec_addec" : "vec_subec";
-      error ("builtin %qs only accepts 3 arguments", name);
-      *res = resolved;
-      return error_mark_node;
-    }
-
-  tree arg0 = (*arglist)[0];
-  tree arg0_type = TREE_TYPE (arg0);
-  tree arg1 = (*arglist)[1];
-  tree arg1_type = TREE_TYPE (arg1);
-  tree arg2 = (*arglist)[2];
-  tree arg2_type = TREE_TYPE (arg2);
 
   /* All 3 arguments must be vectors of (signed or unsigned) (int or
      __int128) and the types must be compatible.  */
-  if (TREE_CODE (arg0_type) != VECTOR_TYPE
-      || !lang_hooks.types_compatible_p (arg0_type, arg1_type)
-      || !lang_hooks.types_compatible_p (arg1_type, arg2_type))
+  if (TREE_CODE (types[0]) != VECTOR_TYPE
+      || !lang_hooks.types_compatible_p (types[0], types[1])
+      || !lang_hooks.types_compatible_p (types[1], types[2]))
     {
       *res = resolved_bad;
       return error_mark_node;
     }
 
-  switch (TYPE_MODE (TREE_TYPE (arg0_type)))
+  switch (TYPE_MODE (TREE_TYPE (types[0])))
     {
       /* For {un}signed ints,
 	   vec_addec (va, vb, carryv) ==
@@ -1224,11 +1168,11 @@  resolve_vec_addec_subec (resolution *res, rs6000_gen_builtins fcode,
       {
 	/* Use save_expr to ensure that operands used more than once that may
 	   have side effects (like calls) are only evaluated once.  */
-	arg0 = save_expr (arg0);
-	arg1 = save_expr (arg1);
+	args[0] = save_expr (args[0]);
+	args[1] = save_expr (args[1]);
 	vec<tree, va_gc> *params = make_tree_vector ();
-	vec_safe_push (params, arg0);
-	vec_safe_push (params, arg1);
+	vec_safe_push (params, args[0]);
+	vec_safe_push (params, args[1]);
 
 	tree as_c_builtin;
 	if (fcode == RS6000_OVLD_VEC_ADDEC)
@@ -1239,8 +1183,8 @@  resolve_vec_addec_subec (resolution *res, rs6000_gen_builtins fcode,
 	tree call1 = altivec_resolve_overloaded_builtin (loc, as_c_builtin,
 							 params);
 	params = make_tree_vector ();
-	vec_safe_push (params, arg0);
-	vec_safe_push (params, arg1);
+	vec_safe_push (params, args[0]);
+	vec_safe_push (params, args[1]);
 
 	tree as_builtin;
 	if (fcode == RS6000_OVLD_VEC_ADDEC)
@@ -1250,10 +1194,10 @@  resolve_vec_addec_subec (resolution *res, rs6000_gen_builtins fcode,
 
 	tree call2 = altivec_resolve_overloaded_builtin (loc, as_builtin,
 							 params);
-	tree const1 = build_int_cstu (TREE_TYPE (arg0_type), 1);
-	tree ones_vector = build_vector_from_val (arg0_type, const1);
-	tree and_expr = fold_build2_loc (loc, BIT_AND_EXPR, arg0_type,
-					 arg2, ones_vector);
+	tree const1 = build_int_cstu (TREE_TYPE (types[0]), 1);
+	tree ones_vector = build_vector_from_val (types[0], const1);
+	tree and_expr = fold_build2_loc (loc, BIT_AND_EXPR, types[0],
+					 args[2], ones_vector);
 	params = make_tree_vector ();
 	vec_safe_push (params, call2);
 	vec_safe_push (params, and_expr);
@@ -1783,78 +1727,15 @@  altivec_resolve_overloaded_builtin (location_t loc, tree fndecl,
 	     "%<vec_lvsr%> is deprecated for little endian; use "
 	     "assignment for unaligned loads and stores");
 
-  /* Some overloads require special handling.  */
-  /* FIXME: Could we simplify the helper functions if we gathered arguments
-     and types into arrays first?  */
-  tree returned_expr = NULL;
-  resolution res = unresolved;
-  vec<tree, va_gc> *arglist = static_cast<vec<tree, va_gc> *> (passed_arglist);
-  unsigned int nargs = vec_safe_length (arglist);
-
-  switch (fcode)
-    {
-    case RS6000_OVLD_VEC_MUL:
-      returned_expr = resolve_vec_mul (&res, arglist, nargs, loc);
-      break;
-
-    case RS6000_OVLD_VEC_CMPNE:
-      returned_expr = resolve_vec_cmpne (&res, arglist, nargs, loc);
-      break;
-
-    case RS6000_OVLD_VEC_ADDE:
-    case RS6000_OVLD_VEC_SUBE:
-      returned_expr = resolve_vec_adde_sube (&res, fcode, arglist, nargs, loc);
-      break;
-
-    case RS6000_OVLD_VEC_ADDEC:
-    case RS6000_OVLD_VEC_SUBEC:
-      returned_expr = resolve_vec_addec_subec (&res, fcode, arglist, nargs,
-					       loc);
-      break;
-
-    case RS6000_OVLD_VEC_SPLATS:
-    case RS6000_OVLD_VEC_PROMOTE:
-      returned_expr = resolve_vec_splats (&res, fcode, arglist, nargs);
-      break;
-
-    case RS6000_OVLD_VEC_EXTRACT:
-      returned_expr = resolve_vec_extract (&res, arglist, nargs, loc);
-      break;
-
-    case RS6000_OVLD_VEC_INSERT:
-      returned_expr = resolve_vec_insert (&res, arglist, nargs, loc);
-      break;
-
-    case RS6000_OVLD_VEC_STEP:
-      returned_expr = resolve_vec_step (&res, arglist, nargs);
-      break;
-
-    default:
-      ;
-    }
-
-  if (res == resolved)
-    return returned_expr;
-
-  /* "Regular" built-in functions and overloaded functions share a namespace
-     for some arrays, like rs6000_builtin_decls.  But rs6000_overload_info
-     only has information for the overloaded functions, so we need an
-     adjusted index for that.  */
-  unsigned int adj_fcode = fcode - RS6000_OVLD_NONE;
-
-  if (res == resolved_bad)
-    {
-      const char *name = rs6000_overload_info[adj_fcode].ovld_name;
-      error ("invalid parameter combination for AltiVec intrinsic %qs", name);
-      return error_mark_node;
-    }
-
   /* Gather the arguments and their types into arrays for easier handling.  */
   tree fnargs = TYPE_ARG_TYPES (TREE_TYPE (fndecl));
   tree types[MAX_OVLD_ARGS];
   tree args[MAX_OVLD_ARGS];
   unsigned int n;
 
+  vec<tree, va_gc> *arglist = static_cast<vec<tree, va_gc> *> (passed_arglist);
+  unsigned int nargs = vec_safe_length (arglist);
+
   for (n = 0;
        !VOID_TYPE_P (TREE_VALUE (fnargs)) && n < nargs;
        fnargs = TREE_CHAIN (fnargs), n++)
@@ -1915,10 +1796,79 @@  altivec_resolve_overloaded_builtin (location_t loc, tree fndecl,
     }
 
   /* If the number of arguments did not match the prototype, return NULL
-     and the generic code will issue the appropriate error message.  */
-  if (!VOID_TYPE_P (TREE_VALUE (fnargs)) || n < nargs)
+     and the generic code will issue the appropriate error message.  Skip
+     this test for functions where we don't fully describe all the possible
+     overload signatures in rs6000-overload.def (because they aren't relevant
+     to the expansion here).  If we don't, we get confusing error messages.  */
+  if (fcode != RS6000_OVLD_VEC_PROMOTE
+      && fcode != RS6000_OVLD_VEC_SPLATS
+      && fcode != RS6000_OVLD_VEC_EXTRACT
+      && fcode != RS6000_OVLD_VEC_INSERT
+      && fcode != RS6000_OVLD_VEC_STEP
+      && (!VOID_TYPE_P (TREE_VALUE (fnargs)) || n < nargs))
     return NULL;
 
+  /* Some overloads require special handling.  */
+  tree returned_expr = NULL;
+  resolution res = unresolved;
+
+  switch (fcode)
+    {
+    case RS6000_OVLD_VEC_MUL:
+      returned_expr = resolve_vec_mul (&res, args, types, loc);
+      break;
+
+    case RS6000_OVLD_VEC_CMPNE:
+      returned_expr = resolve_vec_cmpne (&res, args, types, loc);
+      break;
+
+    case RS6000_OVLD_VEC_ADDE:
+    case RS6000_OVLD_VEC_SUBE:
+      returned_expr = resolve_vec_adde_sube (&res, fcode, args, types, loc);
+      break;
+
+    case RS6000_OVLD_VEC_ADDEC:
+    case RS6000_OVLD_VEC_SUBEC:
+      returned_expr = resolve_vec_addec_subec (&res, fcode, args, types, loc);
+      break;
+
+    case RS6000_OVLD_VEC_SPLATS:
+    case RS6000_OVLD_VEC_PROMOTE:
+      returned_expr = resolve_vec_splats (&res, fcode, arglist, nargs);
+      break;
+
+    case RS6000_OVLD_VEC_EXTRACT:
+      returned_expr = resolve_vec_extract (&res, arglist, nargs, loc);
+      break;
+
+    case RS6000_OVLD_VEC_INSERT:
+      returned_expr = resolve_vec_insert (&res, arglist, nargs, loc);
+      break;
+
+    case RS6000_OVLD_VEC_STEP:
+      returned_expr = resolve_vec_step (&res, arglist, nargs);
+      break;
+
+    default:
+      ;
+    }
+
+  if (res == resolved)
+    return returned_expr;
+
+  /* "Regular" built-in functions and overloaded functions share a namespace
+     for some arrays, like rs6000_builtin_decls.  But rs6000_overload_info
+     only has information for the overloaded functions, so we need an
+     adjusted index for that.  */
+  unsigned int adj_fcode = fcode - RS6000_OVLD_NONE;
+
+  if (res == resolved_bad)
+    {
+      const char *name = rs6000_overload_info[adj_fcode].ovld_name;
+      error ("invalid parameter combination for AltiVec intrinsic %qs", name);
+      return error_mark_node;
+    }
+
   bool unsupported_builtin = false;
   rs6000_gen_builtins instance_code;
   bool supported = false;