From patchwork Mon May 26 19:21:43 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Richard Sandiford X-Patchwork-Id: 352617 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 D641E140079 for ; Tue, 27 May 2014 05:22:00 +1000 (EST) 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; q=dns; s=default; b=WlcN7IYUY+VeaC5E skI9Zef/7skSEnOg16ez/FZN5KnWqj3J4JQF0TCok22rJBDAC64s6GOZ3P9pyz0n LElaIWUF25mKPf9k7szA4Y3JZaCtV9AC5vejB6YEQMG+wDgh9AyY2p739Pr3qF1l aU7/5j7prpNj2sutL4AvomC4zN0= 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; s=default; bh=ofTiZRGyKmPhBj8zM69Frp uIPXM=; b=PL1rsCElRmX7nLtQZC1Xzn9TpB3vLM07Yy4xSgLNDvMI+q+3Afwd6Y +h0W+enC8ugNDS+dswNc1XD40EP8FMMvSvjYljk6aixYHcF/15WwpsNG4IhPLsiw to3KhHHsMMcfyu1+6ZwrXzhQMlv0lYO8fzqWwpBuheQKcGA4ZxeMs= Received: (qmail 20181 invoked by alias); 26 May 2014 19:21:51 -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 20171 invoked by uid 89); 26 May 2014 19:21:50 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.3 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-wi0-f181.google.com Received: from mail-wi0-f181.google.com (HELO mail-wi0-f181.google.com) (209.85.212.181) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Mon, 26 May 2014 19:21:48 +0000 Received: by mail-wi0-f181.google.com with SMTP id n15so484239wiw.8 for ; Mon, 26 May 2014 12:21:45 -0700 (PDT) X-Received: by 10.180.87.165 with SMTP id az5mr31101229wib.10.1401132105036; Mon, 26 May 2014 12:21:45 -0700 (PDT) Received: from localhost ([2.26.169.52]) by mx.google.com with ESMTPSA id mw4sm2109780wib.12.2014.05.26.12.21.43 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 26 May 2014 12:21:44 -0700 (PDT) From: Richard Sandiford To: Jeff Law Mail-Followup-To: Jeff Law , gcc-patches@gcc.gnu.org, rdsandiford@googlemail.com Cc: gcc-patches@gcc.gnu.org Subject: Require '%' to be at the beginning of a constraint string References: <87egzokglh.fsf@talisman.default> <537B9911.7070604@redhat.com> Date: Mon, 26 May 2014 20:21:43 +0100 In-Reply-To: <537B9911.7070604@redhat.com> (Jeff Law's message of "Tue, 20 May 2014 12:04:01 -0600") Message-ID: <87mwe4e46w.fsf_-_@talisman.default> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 Jeff Law writes: > On 05/20/14 02:19, Richard Sandiford wrote: >> On the subject of commutativity, we have: >> >> static bool >> commutative_constraint_p (const char *str) >> { >> int curr_alt, c; >> bool ignore_p; >> >> for (ignore_p = false, curr_alt = 0;;) >> { >> c = *str; >> if (c == '\0') >> break; >> str += CONSTRAINT_LEN (c, str); >> if (c == '#' || !recog_data.alternative_enabled_p[curr_alt]) >> ignore_p = true; >> else if (c == ',') >> { >> curr_alt++; >> ignore_p = false; >> } >> else if (! ignore_p) >> { >> /* Usually `%' is the first constraint character but the >> documentation does not require this. */ >> if (c == '%') >> return true; >> } >> } >> return false; >> } >> >> Any objections to requiring `%' to be the first constraint character? >> Seems wasteful to be searching the constraint string just to support >> an odd case. > If we're going to change it, then clearly the docs need to change and > ideally we'd statically check the port's constraints during the build > process to ensure they meet the tighter definition. OK, how does this look? I built a cc1 for one target per config/ directory to (try to) check that there were no remaining cases. This means that we will silently ignore '%'s that are embedded in the middle of an asm constraint string, but in a way that's already true for most places that check for commutativity. An error seems a bit extreme when '%' is only a hint. If we want a warning, what should the option be called? And should it be under -Wall, -Wextra, or on by default? Tested on x86_64-linux-gnu. OK to install? Thanks, Richard gcc/ * doc/md.texi: Document that the % constraint character must be at the beginning of the string. * genoutput.c (validate_insn_alternatives): Check that '=', '+' and '%' only appear at the beginning of a constraint. * ira.c (commutative_constraint_p): Delete. (ira_get_dup_out_num): Expect the '%' commutativity marker to be at the start of the string. * config/alpha/alpha.md (*movmemdi_1, *clrmemdi_1): Remove duplicate '='s. * config/arm/neon.md (bicdi3_neon): Likewise. * config/vax/vax.md (sbcdi3): Likewise. * config/h8300/h8300.md (*cmpstz): Remove duplicate '+'. * config/arc/arc.md (mulsi_600, mulsidi_600, umulsidi_600) (mul64): Move '%' to beginning of constraint. * config/arm/arm.md (*xordi3_insn): Likewise. * config/nds32/nds32.md (add3, mulsi3, andsi3, iorsi3) (xorsi3): Likewise. Index: gcc/doc/md.texi =================================================================== --- gcc/doc/md.texi 2014-05-26 19:47:27.560682653 +0100 +++ gcc/doc/md.texi 2014-05-26 20:08:23.808948838 +0100 @@ -1589,7 +1589,9 @@ See, for example, the @samp{mulsi3} insn Declares the instruction to be commutative for this operand and the following operand. This means that the compiler may interchange the two operands if that is the cheapest way to make all operands fit the -constraints. +constraints. @samp{%} applies to all alternatives and must appear as +the first character in the constraint. + @ifset INTERNALS This is often used in patterns for addition instructions that really have only two operands: the result must go in one of the Index: gcc/genoutput.c =================================================================== --- gcc/genoutput.c 2014-05-26 19:47:27.560682653 +0100 +++ gcc/genoutput.c 2014-05-26 20:08:23.675947636 +0100 @@ -781,6 +781,11 @@ validate_insn_alternatives (struct data for (p = d->operand[start].constraint; (c = *p); p += len) { + if ((c == '%' || c == '=' || c == '+') + && p != d->operand[start].constraint) + error_with_line (d->lineno, + "character '%c' can only be used at the" + " beginning of a constraint string"); #ifdef USE_MD_CONSTRAINTS if (ISSPACE (c) || strchr (indep_constraints, c)) len = 1; Index: gcc/ira.c =================================================================== --- gcc/ira.c 2014-05-26 20:10:28.988080881 +0100 +++ gcc/ira.c 2014-05-26 20:10:32.773115124 +0100 @@ -1769,38 +1769,6 @@ setup_prohibited_mode_move_regs (void) } - -/* Return TRUE if the operand constraint STR is commutative. */ -static bool -commutative_constraint_p (const char *str) -{ - int curr_alt, c; - bool ignore_p; - - for (ignore_p = false, curr_alt = 0;;) - { - c = *str; - if (c == '\0') - break; - str += CONSTRAINT_LEN (c, str); - if (c == '#' || !recog_data.alternative_enabled_p[curr_alt]) - ignore_p = true; - else if (c == ',') - { - curr_alt++; - ignore_p = false; - } - else if (! ignore_p) - { - /* Usually `%' is the first constraint character but the - documentation does not require this. */ - if (c == '%') - return true; - } - } - return false; -} - /* Setup possible alternatives in ALTS for INSN. */ void ira_setup_alts (rtx insn, HARD_REG_SET &alts) @@ -2101,10 +2069,9 @@ ira_get_dup_out_num (int op_num, HARD_RE if (use_commut_op_p) break; use_commut_op_p = true; - if (commutative_constraint_p (recog_data.constraints[op_num])) + if (recog_data.constraints[op_num][0] == '%') str = recog_data.constraints[op_num + 1]; - else if (op_num > 0 && commutative_constraint_p (recog_data.constraints - [op_num - 1])) + else if (op_num > 0 && recog_data.constraints[op_num - 1][0] == '%') str = recog_data.constraints[op_num - 1]; else break; Index: gcc/config/alpha/alpha.md =================================================================== --- gcc/config/alpha/alpha.md 2014-05-26 19:47:27.560682653 +0100 +++ gcc/config/alpha/alpha.md 2014-05-26 20:08:23.697947834 +0100 @@ -4764,7 +4764,7 @@ (define_expand "movmemdi" "operands[4] = gen_rtx_SYMBOL_REF (Pmode, \"OTS$MOVE\");") (define_insn "*movmemdi_1" - [(set (match_operand:BLK 0 "memory_operand" "=m,=m") + [(set (match_operand:BLK 0 "memory_operand" "=m,m") (match_operand:BLK 1 "memory_operand" "m,m")) (use (match_operand:DI 2 "nonmemory_operand" "r,i")) (use (match_operand:DI 3 "immediate_operand")) @@ -4831,7 +4831,7 @@ (define_expand "setmemdi" }) (define_insn "*clrmemdi_1" - [(set (match_operand:BLK 0 "memory_operand" "=m,=m") + [(set (match_operand:BLK 0 "memory_operand" "=m,m") (const_int 0)) (use (match_operand:DI 1 "nonmemory_operand" "r,i")) (use (match_operand:DI 2 "immediate_operand")) Index: gcc/config/arm/neon.md =================================================================== --- gcc/config/arm/neon.md 2014-05-26 19:47:27.560682653 +0100 +++ gcc/config/arm/neon.md 2014-05-26 20:08:23.720948042 +0100 @@ -728,7 +728,7 @@ (define_insn "bic3_neon" ;; Compare to *anddi_notdi_di. (define_insn "bicdi3_neon" - [(set (match_operand:DI 0 "s_register_operand" "=w,?=&r,?&r") + [(set (match_operand:DI 0 "s_register_operand" "=w,?&r,?&r") (and:DI (not:DI (match_operand:DI 2 "s_register_operand" "w,r,0")) (match_operand:DI 1 "s_register_operand" "w,0,r")))] "TARGET_NEON" Index: gcc/config/vax/vax.md =================================================================== --- gcc/config/vax/vax.md 2014-05-26 19:47:27.560682653 +0100 +++ gcc/config/vax/vax.md 2014-05-26 20:08:23.776948548 +0100 @@ -423,7 +423,7 @@ (define_expand "subdi3" "vax_expand_addsub_di_operands (operands, MINUS); DONE;") (define_insn "sbcdi3" - [(set (match_operand:DI 0 "nonimmediate_addsub_di_operand" "=Rr,=Rr") + [(set (match_operand:DI 0 "nonimmediate_addsub_di_operand" "=Rr,Rr") (minus:DI (match_operand:DI 1 "general_addsub_di_operand" "0,I") (match_operand:DI 2 "general_addsub_di_operand" "nRr,Rr")))] "TARGET_QMATH" Index: gcc/config/h8300/h8300.md =================================================================== --- gcc/config/h8300/h8300.md 2014-05-26 19:47:27.560682653 +0100 +++ gcc/config/h8300/h8300.md 2014-05-26 20:08:23.791948684 +0100 @@ -3589,7 +3589,7 @@ (define_insn "*bstzhireg" [(set_attr "cc" "clobber")]) (define_insn_and_split "*cmpstz" - [(set (zero_extract:QI (match_operand:QI 0 "bit_memory_operand" "+WU,+WU") + [(set (zero_extract:QI (match_operand:QI 0 "bit_memory_operand" "+WU,WU") (const_int 1) (match_operand:QI 1 "immediate_operand" "n,n")) (match_operator:QI 2 "eqne_operator" Index: gcc/config/arc/arc.md =================================================================== --- gcc/config/arc/arc.md 2014-05-26 19:47:27.560682653 +0100 +++ gcc/config/arc/arc.md 2014-05-26 20:08:23.590946867 +0100 @@ -1698,7 +1698,7 @@ (define_insn "mac_600" (define_insn "mulsi_600" [(set (match_operand:SI 2 "mlo_operand" "") - (mult:SI (match_operand:SI 0 "register_operand" "Rcq#q,c,c,%c") + (mult:SI (match_operand:SI 0 "register_operand" "%Rcq#q,c,c,c") (match_operand:SI 1 "nonmemory_operand" "Rcq#q,cL,I,Cal"))) (clobber (match_operand:SI 3 "mhi_operand" ""))] "TARGET_MUL64_SET" @@ -1750,7 +1750,7 @@ (define_insn "mulsi3_600_lib" (define_insn "mulsidi_600" [(set (reg:DI MUL64_OUT_REG) (mult:DI (sign_extend:DI - (match_operand:SI 0 "register_operand" "Rcq#q,c,c,%c")) + (match_operand:SI 0 "register_operand" "%Rcq#q,c,c,c")) (sign_extend:DI ; assembler issue for "I", see mulsi_600 ; (match_operand:SI 1 "register_operand" "Rcq#q,cL,I,Cal"))))] @@ -1766,7 +1766,7 @@ (define_insn "mulsidi_600" (define_insn "umulsidi_600" [(set (reg:DI MUL64_OUT_REG) (mult:DI (zero_extend:DI - (match_operand:SI 0 "register_operand" "c,c,%c")) + (match_operand:SI 0 "register_operand" "%c,c,c")) (sign_extend:DI ; assembler issue for "I", see mulsi_600 ; (match_operand:SI 1 "register_operand" "cL,I,Cal"))))] @@ -4134,7 +4134,7 @@ (define_insn "swap" ;; FIXME: an intrinsic for multiply is daft. Can we remove this? (define_insn "mul64" - [(unspec [(match_operand:SI 0 "general_operand" "q,r,r,%r") + [(unspec [(match_operand:SI 0 "general_operand" "%q,r,r,r") (match_operand:SI 1 "general_operand" "q,rL,I,Cal")] UNSPEC_MUL64)] "TARGET_MUL64_SET" Index: gcc/config/arm/arm.md =================================================================== --- gcc/config/arm/arm.md 2014-05-26 19:47:27.560682653 +0100 +++ gcc/config/arm/arm.md 2014-05-26 20:08:23.629947220 +0100 @@ -3169,7 +3169,7 @@ (define_expand "xordi3" (define_insn_and_split "*xordi3_insn" [(set (match_operand:DI 0 "s_register_operand" "=w,&r,&r,&r,&r,?w") - (xor:DI (match_operand:DI 1 "s_register_operand" "w ,%0,r ,0 ,r ,w") + (xor:DI (match_operand:DI 1 "s_register_operand" "%w ,0,r ,0 ,r ,w") (match_operand:DI 2 "arm_xordi_operand" "w ,r ,r ,Dg,Dg,w")))] "TARGET_32BIT && !TARGET_IWMMXT" { Index: gcc/config/nds32/nds32.md =================================================================== --- gcc/config/nds32/nds32.md 2014-05-26 19:47:27.560682653 +0100 +++ gcc/config/nds32/nds32.md 2014-05-26 20:08:23.663947527 +0100 @@ -261,7 +261,7 @@ (define_insn "extendsi2" (define_insn "add3" [(set (match_operand:QIHISI 0 "register_operand" "= d, l, d, l, d, l, k, l, r, r") - (plus:QIHISI (match_operand:QIHISI 1 "register_operand" " 0, l, 0, l, %0, l, 0, k, r, r") + (plus:QIHISI (match_operand:QIHISI 1 "register_operand" "% 0, l, 0, l, 0, l, 0, k, r, r") (match_operand:QIHISI 2 "nds32_rimm15s_operand" " In05, In03, Iu05, Iu03, r, l, Is10, Iu06, Is15, r")))] "" { @@ -382,9 +382,9 @@ (define_insn "*sub_srli" ;; Multiplication instructions. (define_insn "mulsi3" - [(set (match_operand:SI 0 "register_operand" "= w, r") - (mult:SI (match_operand:SI 1 "register_operand" " %0, r") - (match_operand:SI 2 "register_operand" " w, r")))] + [(set (match_operand:SI 0 "register_operand" "=w, r") + (mult:SI (match_operand:SI 1 "register_operand" "%0, r") + (match_operand:SI 2 "register_operand" " w, r")))] "" "@ mul33\t%0, %2 @@ -489,9 +489,9 @@ (define_insn "bitc" ) (define_insn "andsi3" - [(set (match_operand:SI 0 "register_operand" "= w, r, l, l, l, l, l, l, r, r, r, r, r") - (and:SI (match_operand:SI 1 "register_operand" " %0, r, l, l, l, l, 0, 0, r, r, r, r, r") - (match_operand:SI 2 "general_operand" " w, r, Izeb, Izeh, Ixls, Ix11, Ibms, Ifex, Izeb, Izeh, Iu15, Ii15, Ic15")))] + [(set (match_operand:SI 0 "register_operand" "=w, r, l, l, l, l, l, l, r, r, r, r, r") + (and:SI (match_operand:SI 1 "register_operand" "%0, r, l, l, l, l, 0, 0, r, r, r, r, r") + (match_operand:SI 2 "general_operand" " w, r, Izeb, Izeh, Ixls, Ix11, Ibms, Ifex, Izeb, Izeh, Iu15, Ii15, Ic15")))] "" { HOST_WIDE_INT mask = INTVAL (operands[2]); @@ -585,9 +585,9 @@ (define_insn "*and_srli" ;; For V3/V3M ISA, we have 'or33' instruction. ;; So we can identify 'or Rt3,Rt3,Ra3' case and set its length to be 2. (define_insn "iorsi3" - [(set (match_operand:SI 0 "register_operand" "= w, r, r, r") - (ior:SI (match_operand:SI 1 "register_operand" " %0, r, r, r") - (match_operand:SI 2 "general_operand" " w, r, Iu15, Ie15")))] + [(set (match_operand:SI 0 "register_operand" "=w, r, r, r") + (ior:SI (match_operand:SI 1 "register_operand" "%0, r, r, r") + (match_operand:SI 2 "general_operand" " w, r, Iu15, Ie15")))] "" { int one_position; @@ -645,9 +645,9 @@ (define_insn "*or_srli" ;; For V3/V3M ISA, we have 'xor33' instruction. ;; So we can identify 'xor Rt3,Rt3,Ra3' case and set its length to be 2. (define_insn "xorsi3" - [(set (match_operand:SI 0 "register_operand" "= w, r, r, r") - (xor:SI (match_operand:SI 1 "register_operand" " %0, r, r, r") - (match_operand:SI 2 "general_operand" " w, r, Iu15, It15")))] + [(set (match_operand:SI 0 "register_operand" "=w, r, r, r") + (xor:SI (match_operand:SI 1 "register_operand" "%0, r, r, r") + (match_operand:SI 2 "general_operand" " w, r, Iu15, It15")))] "" { int one_position;