Re: [3/3] Record the number of generator arguments in insn_data

[Richard Sandiford <richard dot sandiford at linaro dot org>]

Richard Henderson <rth@redhat.com> writes:
> On 03/31/2011 01:09 PM, Richard Sandiford wrote:
>>> I think the assert should be retained (for now) as
>>>
>>>   gcc_assert (nops == 4 || nops == 6);
>>>
>>> at least until we add such verification to some genfoo.
>> 
>> After the patch we have:
>> 
>>   gcc_assert (nops == (unsigned int) insn_data[(int) icode].n_generator_args);
>> 
>> in maybe_gen_insn, which should catch this.  Just to check: do you want
>> the assertion here anyway?
>
> Well, it won't because we initialized nops *with .n_generator_args.
>
> If nops was initialized with
>
>   nops = 4;
>   create the first 4 args
>
>   if (.n_generator_args == 6)
>     {
>       nops = 6;
>       create the last two args
>     }
>
> then the assert you mention *would* do the trick.

Ah, yeah.  You're right of course.  Here's what I installed.

Richard


gcc/
	* expr.c (emit_block_move_via_movmem): Use n_generator_args
	instead of n_operands.
	(set_storage_via_setmem): Likewise.
	* optabs.c (maybe_gen_insn): Likewise.
	* config/arm/arm.c (arm_init_neon_builtins): Likewise.
	* config/mips/mips.c (mips_expand_builtin_compare_1): Likewise.
	(mips_expand_builtin_direct): Likewise.
	* config/spu/spu.c (expand_builtin_args): Likewise.

Index: gcc/expr.c
===================================================================
--- gcc/expr.c	2011-04-01 08:43:30.000000000 +0100
+++ gcc/expr.c	2011-04-01 08:44:22.000000000 +0100
@@ -1293,11 +1293,8 @@ emit_block_move_via_movmem (rtx x, rtx y
 	     nice if there were some way to inform the backend, so
 	     that it doesn't fail the expansion because it thinks
 	     emitting the libcall would be more efficient.  */
-	  nops = insn_data[(int) code].n_operands;
-	  /* ??? n_operands includes match_scratches; find some other
-	     way to select the 6 operand variant, or force all targets
-	     to have exactly 6 operands.  */
-	  gcc_assert (nops >= 4 && nops <= 6);
+	  nops = insn_data[(int) code].n_generator_args;
+	  gcc_assert (nops == 4 || nops == 6);
 
 	  create_fixed_operand (&ops[0], x);
 	  create_fixed_operand (&ops[1], y);
@@ -2719,11 +2716,8 @@ set_storage_via_setmem (rtx object, rtx 
 	  struct expand_operand ops[6];
 	  unsigned int nops;
 
-	  nops = insn_data[(int) code].n_operands;
-	  /* ??? n_operands includes match_scratches; find some other
-	     way to select the 6 operand variant, or force all targets
-	     to have exactly 6 operands.  */
-	  gcc_assert (nops >= 4 && nops <= 6);
+	  nops = insn_data[(int) code].n_generator_args;
+	  gcc_assert (nops == 4 || nops == 6);
 
 	  create_fixed_operand (&ops[0], object);
 	  /* The check above guarantees that this size conversion is valid.  */
Index: gcc/optabs.c
===================================================================
--- gcc/optabs.c	2011-04-01 08:43:30.000000000 +0100
+++ gcc/optabs.c	2011-04-01 08:43:32.000000000 +0100
@@ -7149,9 +7149,7 @@ maybe_legitimize_operands (enum insn_cod
 maybe_gen_insn (enum insn_code icode, unsigned int nops,
 		struct expand_operand *ops)
 {
-  /* n_operands includes any automatically-generated match_scratches,
-     so we can't check for equality here.  */
-  gcc_assert (nops <= (unsigned int) insn_data[(int) icode].n_operands);
+  gcc_assert (nops == (unsigned int) insn_data[(int) icode].n_generator_args);
   if (!maybe_legitimize_operands (icode, 0, nops, ops))
     return NULL_RTX;
 
Index: gcc/config/arm/arm.c
===================================================================
--- gcc/config/arm/arm.c	2011-04-01 08:43:30.000000000 +0100
+++ gcc/config/arm/arm.c	2011-04-01 08:43:32.000000000 +0100
@@ -18944,7 +18944,7 @@ arm_init_neon_builtins (void)
 		/* Build a function type directly from the insn_data for this
 		   builtin.  The build_function_type() function takes care of
 		   removing duplicates for us.  */
-		for (k = insn_data[icode].n_operands - 1; k >= 0; k--)
+		for (k = insn_data[icode].n_generator_args - 1; k >= 0; k--)
 		  {
 		    tree eltype;
 
Index: gcc/config/mips/mips.c
===================================================================
--- gcc/config/mips/mips.c	2011-04-01 08:43:30.000000000 +0100
+++ gcc/config/mips/mips.c	2011-04-01 08:43:32.000000000 +0100
@@ -13252,7 +13252,7 @@ mips_expand_builtin_compare_1 (enum insn
 
   /* The instruction should have a target operand, an operand for each
      argument, and an operand for COND.  */
-  gcc_assert (nargs + 2 == insn_data[(int) icode].n_operands);
+  gcc_assert (nargs + 2 == insn_data[(int) icode].n_generator_args);
 
   opno = 0;
   create_output_operand (&ops[opno++], NULL_RTX,
@@ -13280,11 +13280,9 @@ mips_expand_builtin_direct (enum insn_co
   if (has_target_p)
     create_output_operand (&ops[opno++], target, TYPE_MODE (TREE_TYPE (exp)));
 
-  /* Map the arguments to the other operands.  The n_operands value
-     for an expander includes match_dups and match_scratches as well as
-     match_operands, so n_operands is only an upper bound on the number
-     of arguments to the expander function.  */
-  gcc_assert (opno + call_expr_nargs (exp) <= insn_data[icode].n_operands);
+  /* Map the arguments to the other operands.  */
+  gcc_assert (opno + call_expr_nargs (exp)
+	      == insn_data[icode].n_generator_args);
   for (argno = 0; argno < call_expr_nargs (exp); argno++)
     mips_prepare_builtin_arg (&ops[opno++], exp, argno);
 
Index: gcc/config/spu/spu.c
===================================================================
--- gcc/config/spu/spu.c	2011-04-01 08:43:30.000000000 +0100
+++ gcc/config/spu/spu.c	2011-04-01 08:43:32.000000000 +0100
@@ -6545,9 +6545,7 @@ expand_builtin_args (struct spu_builtin_
       ops[i] = expand_expr (arg, NULL_RTX, VOIDmode, EXPAND_NORMAL);
     }
 
-  /* The insn pattern may have additional operands (SCRATCH).
-     Return the number of actual non-SCRATCH operands.  */
-  gcc_assert (i <= insn_data[icode].n_operands);
+  gcc_assert (i == insn_data[icode].n_generator_args);
   return i;
 }