From patchwork Fri Apr 1 08:20:18 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Richard Sandiford X-Patchwork-Id: 89240 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id 30924B6F80 for ; Fri, 1 Apr 2011 19:20:38 +1100 (EST) Received: (qmail 8341 invoked by alias); 1 Apr 2011 08:20:33 -0000 Received: (qmail 8330 invoked by uid 22791); 1 Apr 2011 08:20:30 -0000 X-SWARE-Spam-Status: No, hits=-2.4 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_LOW X-Spam-Check-By: sourceware.org Received: from mail-wy0-f175.google.com (HELO mail-wy0-f175.google.com) (74.125.82.175) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 01 Apr 2011 08:20:24 +0000 Received: by wye20 with SMTP id 20so3179921wye.20 for ; Fri, 01 Apr 2011 01:20:22 -0700 (PDT) Received: by 10.216.141.72 with SMTP id f50mr3642206wej.26.1301646022525; Fri, 01 Apr 2011 01:20:22 -0700 (PDT) Received: from richards-thinkpad (gbibp9ph1--blueice2n1.emea.ibm.com [195.212.29.75]) by mx.google.com with ESMTPS id m2sm872173wer.13.2011.04.01.01.20.20 (version=TLSv1/SSLv3 cipher=OTHER); Fri, 01 Apr 2011 01:20:20 -0700 (PDT) From: Richard Sandiford To: Richard Henderson Mail-Followup-To: Richard Henderson , gcc-patches@gcc.gnu.org, patches@linaro.org, richard.sandiford@linaro.org Cc: gcc-patches@gcc.gnu.org, patches@linaro.org Subject: Re: [3/3] Record the number of generator arguments in insn_data References: <4D94DA43.10108@redhat.com> <87y63vnjbe.fsf@firetop.home> <4D94E02B.1080709@redhat.com> Date: Fri, 01 Apr 2011 09:20:18 +0100 In-Reply-To: <4D94E02B.1080709@redhat.com> (Richard Henderson's message of "Thu, 31 Mar 2011 13:12:27 -0700") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.1 (gnu/linux) MIME-Version: 1.0 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Richard Henderson 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; }