From patchwork Thu May 7 13:59:56 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Richard Sandiford X-Patchwork-Id: 469701 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]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 1D8581402C4 for ; Fri, 8 May 2015 00:00:31 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=osPzApbo; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :to:cc:subject:references:date:in-reply-to:message-id :mime-version:content-type:content-transfer-encoding; q=dns; s= default; b=ApLvfb9LIH3vSQcCF9STibqSpH/M/o88vnR+7K9Y0E963Wg64GSvw QxbBr6Le1SWdLRkJHtmpft2ENE/8h6cJZdWhTpfkTYl3nGNl3GXUqQXUcOFeu9C0 YyVq2QywKB2EeMS+j/vkpcJifeZttXCWFK9F4JNYODwk3PShz1wVQs= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :to:cc:subject:references:date:in-reply-to:message-id :mime-version:content-type:content-transfer-encoding; s=default; bh=Tel5kWr60e0BOjSBKTLT3fLKpB0=; b=osPzApboPbeiMY/eCKbSCTa7S9EI 0o8pzcFHJw5wyXCcla7n015Rt9j90iPX9/5rdOPvM8cC86Ux5y4kPdA7h3Dd4fGC rw9BrrevyJ5KGW+awCJaY2+QfI0xcZkP0rVAN3ozUxk8XjgJPSWRlK9vq62SlVhq jehnMg45jT/wmRQ= Received: (qmail 127540 invoked by alias); 7 May 2015 14:00:07 -0000 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 Received: (qmail 127432 invoked by uid 89); 7 May 2015 14:00:06 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.4 required=5.0 tests=AWL, BAYES_00, KAM_ASCII_DIVIDERS, SPF_PASS autolearn=no version=3.3.2 X-HELO: eu-smtp-delivery-143.mimecast.com Received: from eu-smtp-delivery-143.mimecast.com (HELO eu-smtp-delivery-143.mimecast.com) (207.82.80.143) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 07 May 2015 14:00:04 +0000 Received: from cam-owa1.Emea.Arm.com (fw-tnat.cambridge.arm.com [217.140.96.140]) by uk-mta-18.uk.mimecast.lan; Thu, 07 May 2015 14:59:57 +0100 Received: from localhost ([10.1.2.79]) by cam-owa1.Emea.Arm.com with Microsoft SMTPSVC(6.0.3790.3959); Thu, 7 May 2015 14:59:57 +0100 From: Richard Sandiford To: Jeff Law Mail-Followup-To: Jeff Law , gcc-patches@gcc.gnu.org, richard.sandiford@arm.com Cc: gcc-patches@gcc.gnu.org Subject: Re: Remove mode argument from gen_rtx_SET References: <87ioc4vct9.fsf@e105548-lin.cambridge.arm.com> <554B6A5F.8050105@redhat.com> Date: Thu, 07 May 2015 14:59:56 +0100 In-Reply-To: <554B6A5F.8050105@redhat.com> (Jeff Law's message of "Thu, 07 May 2015 07:36:31 -0600") Message-ID: <87egmsv677.fsf@e105548-lin.cambridge.arm.com> User-Agent: Gnus/5.130012 (Ma Gnus v0.12) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 X-MC-Unique: JWLNpSPLStaqLf7Igtpmdg-1 Jeff Law writes: > On 05/07/2015 05:37 AM, Richard Sandiford wrote: >> One problem with the automatically-generated gen_rtx_FOO () macros >> is that they always have a mode parameter, even for codes like SET >> where the mode should always be VOIDmode. This inevitably leads to >> cases where a caller accidentally passes something other than VOIDmode. >> E.g. when expanding an SImode move, the temptation is to make everything >> SImode, even the SETs. This in turn can cause two instructions to appear >> different simply because their SETs have different modes, even though the >> SET_DEST and SET_SRC are identical. >> >> E.g. for gcc/testsuite/g++.dg/torture/pr34651.C on lm32-elf we have >> the following before jump2: >> >> (jump_insn 42 191 43 5 (set (pc) >> (if_then_else (eq:SI (reg:SI 13 r13 [orig:43 inHotKey$4+-3 ] [43]) >> (const_int 0 [0])) >> (label_ref 53) >> (pc))) gcc/testsuite/g++.dg/torture/pr34651.C:22 22 {*beq} >> (expr_list:REG_DEAD (reg:SI 13 r13 [orig:43 inHotKey$4+-3 ] [43]) >> (int_list:REG_BR_PROB 5000 (nil))) >> -> 53) >> (note 43 42 48 6 [bb 6] NOTE_INSN_BASIC_BLOCK) >> (insn 48 43 47 6 (set (reg:SI 2 r2) >> (mem/u/c:SI (reg:SI 1 r1) [4 S4 A32])) gcc/testsuite/g++.dg/torture/pr34651.C:22 7 {movsi_insn} >> (expr_list:REG_DEAD (reg:SI 1 r1) >> (nil))) >> [...] >> (code_label 53 169 54 7 6 "" [1 uses]) >> (note 54 53 12 7 [bb 7] NOTE_INSN_BASIC_BLOCK) >> (insn 12 54 57 7 (set:SI (reg/f:SI 2 r2 [orig:46 D.2050 ] [46]) >> (mem/u/c:SI (reg:SI 1 r1) [4 S4 A32])) gcc/testsuite/g++.dg/torture/pr34651.C:22 7 {movsi_insn} >> (expr_list:REG_DEAD (reg:SI 1 r1) >> (expr_list:REG_EQUAL (symbol_ref/f:SI ("*.LC3") [flags 0x2] ) >> (nil)))) >> >> where insns 12 and 48 are identical except for the :SI on the SET. >> This difference prevents us from merging the heads of the two blocks; >> after removing it we replace the two loads with a single load before >> the branch. >> >> This patch removes the mode argument from gen_rtx_SET and updates >> all callers. I used a script to (try to) make sure that all callers >> really had been caught. I also built one target per CPU just in case. >> There were some changes in gcc.dg, g++.dg and gcc.c-torture assembly >> code for c6x-elf, lm32-elf and v850-elf, but all of them seemed to be >> code improvements from removing duplicated instructions. (Other ports >> also passed spurious modes but apparently not in a way that affects >> the tests I'd tried.) Also tested on x86_64-linux-gnu. OK to install? >> >> BTW, I've split the patch up into two, the last bit being a mechanical >> removal of modes. (I did it by hand though to try to keep things >> properly formatted.) >> >> Thanks, >> Richard >> >> >> gcc/ >> * rtl.h (always_void_p): New function. >> * gengenrtl.c (always_void_p(: Likewise. > Typo in the ChangeLog. > > non-mechanical part didn't seem to be attached. I don't expect any > problems there, but a quick looksie seems appropriate. Bah. Sorry about that. It's unfortunate that we have two copies of the test -- one on codes and one on strings -- but that's how everything else in gengenrtl.c is done. Maybe a later clean-up. Index: gcc/genemit.c =================================================================== --- gcc/genemit.c 2015-05-07 07:59:47.890577327 +0100 +++ gcc/genemit.c 2015-05-07 07:59:47.874577523 +0100 @@ -94,6 +94,7 @@ gen_exp (rtx x, enum rtx_code subroutine int i; int len; const char *fmt; + const char *sep = ""; if (x == 0) { @@ -215,7 +216,12 @@ gen_exp (rtx x, enum rtx_code subroutine printf ("gen_rtx_"); print_code (code); - printf (" (%smode", GET_MODE_NAME (GET_MODE (x))); + printf (" ("); + if (!always_void_p (code)) + { + printf ("%smode", GET_MODE_NAME (GET_MODE (x))); + sep = ",\n\t"; + } fmt = GET_RTX_FORMAT (code); len = GET_RTX_LENGTH (code); @@ -223,7 +229,7 @@ gen_exp (rtx x, enum rtx_code subroutine { if (fmt[i] == '0') break; - printf (",\n\t"); + fputs (sep, stdout); switch (fmt[i]) { case 'e': case 'u': @@ -254,6 +260,7 @@ gen_exp (rtx x, enum rtx_code subroutine default: gcc_unreachable (); } + sep = ",\n\t"; } printf (")"); } Index: gcc/gengenrtl.c =================================================================== --- gcc/gengenrtl.c 2015-05-07 07:59:47.890577327 +0100 +++ gcc/gengenrtl.c 2015-05-07 08:12:24.281310491 +0100 @@ -116,6 +116,14 @@ special_format (const char *fmt) || strchr (fmt, 'n') != 0); } +/* Return true if CODE always has VOIDmode. */ + +static inline bool +always_void_p (int idx) +{ + return strcmp (defs[idx].enumname, "SET") == 0; +} + /* Return nonzero if the RTL code given by index IDX is one that we should generate a gen_rtx_raw_FOO macro for, not gen_rtx_FOO (because gen_rtx_FOO is a wrapper in emit-rtl.c). */ @@ -181,6 +189,7 @@ find_formats (void) genmacro (int idx) { const char *p; + const char *sep = ""; int i; /* We write a macro that defines gen_rtx_RTLCODE to be an equivalent to @@ -190,15 +199,25 @@ genmacro (int idx) /* Don't define a macro for this code. */ return; - printf ("#define gen_rtx_%s%s(MODE", + bool has_mode_p = !always_void_p (idx); + printf ("#define gen_rtx_%s%s(", special_rtx (idx) ? "raw_" : "", defs[idx].enumname); + if (has_mode_p) + { + printf ("MODE"); + sep = ", "; + } for (p = defs[idx].format, i = 0; *p != 0; p++) if (*p != '0') - printf (", ARG%d", i++); - - printf (") \\\n gen_rtx_fmt_%s (%s, (MODE)", - defs[idx].format, defs[idx].enumname); + { + printf ("%sARG%d", sep, i++); + sep = ", "; + } + + printf (") \\\n gen_rtx_fmt_%s (%s, %s", + defs[idx].format, defs[idx].enumname, + has_mode_p ? "(MODE)" : "VOIDmode"); for (p = defs[idx].format, i = 0; *p != 0; p++) if (*p != '0') Index: gcc/rtl.h =================================================================== --- gcc/rtl.h 2015-05-07 07:59:47.890577327 +0100 +++ gcc/rtl.h 2015-05-07 08:11:56.405652046 +0100 @@ -1787,6 +1787,14 @@ #define COSTS_N_INSNS(N) ((N) * 4) not to use an rtx with this cost under any circumstances. */ #define MAX_COST INT_MAX +/* Return true if CODE always has VOIDmode. */ + +static inline bool +always_void_p (enum rtx_code code) +{ + return code == SET; +} + /* A structure to hold all available cost information about an rtl expression. */ struct full_rtx_costs @@ -3597,5 +3605,4 @@ #define fatal_insn_not_found(insn) \ /* reginfo.c */ extern tree GTY(()) global_regs_decl[FIRST_PSEUDO_REGISTER]; - #endif /* ! GCC_RTL_H */