From patchwork Sun Jun 20 15:25:58 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jakub Jelinek X-Patchwork-Id: 56265 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 3CAC51007D5 for ; Mon, 21 Jun 2010 01:25:49 +1000 (EST) Received: (qmail 24424 invoked by alias); 20 Jun 2010 15:25:47 -0000 Received: (qmail 24403 invoked by uid 22791); 20 Jun 2010 15:25:46 -0000 X-SWARE-Spam-Status: No, hits=-6.0 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_HI, SPF_HELO_PASS, T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sun, 20 Jun 2010 15:25:38 +0000 Received: from int-mx05.intmail.prod.int.phx2.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.18]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id o5KFPZLN010874 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Sun, 20 Jun 2010 11:25:36 -0400 Received: from tyan-ft48-01.lab.bos.redhat.com (tyan-ft48-01.lab.bos.redhat.com [10.16.42.4]) by int-mx05.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id o5KFPZDu000661 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Sun, 20 Jun 2010 11:25:35 -0400 Received: from tyan-ft48-01.lab.bos.redhat.com (tyan-ft48-01.lab.bos.redhat.com [127.0.0.1]) by tyan-ft48-01.lab.bos.redhat.com (8.14.4/8.14.4) with ESMTP id o5KFQ1ZH017313; Sun, 20 Jun 2010 17:26:01 +0200 Received: (from jakub@localhost) by tyan-ft48-01.lab.bos.redhat.com (8.14.4/8.14.4/Submit) id o5KFPwc4017311; Sun, 20 Jun 2010 17:25:58 +0200 Date: Sun, 20 Jun 2010 17:25:58 +0200 From: Jakub Jelinek To: Ian Lance Taylor , Hans-Peter Nilsson , Richard Guenther Cc: gcc-patches@gcc.gnu.org Subject: [PATCH] Disallow side-effects in inline asm operands unless < or > constraint is used (PR middle-end/44492) Message-ID: <20100620152558.GL7811@tyan-ft48-01.lab.bos.redhat.com> Reply-To: Jakub Jelinek References: <20100611180013.GC7811@tyan-ft48-01.lab.bos.redhat.com> <20100620115041.GJ7811@tyan-ft48-01.lab.bos.redhat.com> <20100620140013.GK7811@tyan-ft48-01.lab.bos.redhat.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.20 (2009-12-10) X-IsSubscribed: yes 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 On Sun, Jun 20, 2010 at 11:16:15AM -0400, Hans-Peter Nilsson wrote: > On Sun, 20 Jun 2010, Jakub Jelinek wrote: > > Anyway, I guess I can live with not adding _ and saying that side-effects in > > inline-asm memory is not allowed unless the constraint contains < or >. > > Then PRE_MODIFY resp. POST_MODIFY would be allowed in > > "m<>" or even just "m<" or "m>", assuming PRE_MODIFY resp. POST_MODIFY > > is allowed in memory_address. > > > > The implementation would be roughly the same as the patch I've posted, just > > would set incdec_ok instead of _. > > And a much shorter documentation patch! :) > (I guess mentioning that asm operands will not have side-effects > unless < or > are explicitly mentioned would be right.) > > > Advantage of doing it that way is backwards compatibility, "m<>" can be used > > even with older gccs. > > > > If you prefer to do it that way, I can write a patch. > > IMHO that's preferable, unless there's a known specific need for > what "_m" brings in addition to "m<>" as suggested above. Here is a new patch. Ok for trunk? 2010-06-20 Jakub Jelinek PR middle-end/44492 * recog.h (struct recog_data): Add is_asm field. * recog.c (asm_operand_ok, constrain_operands): If neither < nor > is present in constraints of inline-asm operand and memory operand contains {PRE,POST}_{INC,DEC,MODIFY}, return 0. (extract_insn): Initialize recog_data.is_asm. * doc/md.texi (Constraints): Document operand side-effect rules. * g++.dg/torture/pr44492.C: New test. Jakub --- gcc/recog.h.jj 2010-06-14 17:14:16.481241913 +0200 +++ gcc/recog.h 2010-06-20 16:15:01.218441495 +0200 @@ -230,6 +230,9 @@ struct recog_data /* The number of alternatives in the constraints for the insn. */ char n_alternatives; + /* True if insn is ASM_OPERANDS. */ + bool is_asm; + /* Specifies whether an insn alternative is enabled using the `enabled' attribute in the insn pattern definition. For back ends not using the `enabled' attribute the array fields are --- gcc/recog.c.jj 2010-06-14 17:14:16.466197350 +0200 +++ gcc/recog.c 2010-06-20 16:17:10.676210868 +0200 @@ -1601,6 +1601,9 @@ int asm_operand_ok (rtx op, const char *constraint, const char **constraints) { int result = 0; +#ifdef AUTO_INC_DEC + bool incdec_ok = false; +#endif /* Use constrain_operands after reload. */ gcc_assert (!reload_completed); @@ -1608,7 +1611,7 @@ asm_operand_ok (rtx op, const char *cons /* Empty constraint string is the same as "X,...,X", i.e. X for as many alternatives as required to match the other operands. */ if (*constraint == '\0') - return 1; + result = 1; while (*constraint) { @@ -1685,6 +1688,9 @@ asm_operand_ok (rtx op, const char *cons || GET_CODE (XEXP (op, 0)) == PRE_DEC || GET_CODE (XEXP (op, 0)) == POST_DEC)) result = 1; +#ifdef AUTO_INC_DEC + incdec_ok = true; +#endif break; case '>': @@ -1693,6 +1699,9 @@ asm_operand_ok (rtx op, const char *cons || GET_CODE (XEXP (op, 0)) == PRE_INC || GET_CODE (XEXP (op, 0)) == POST_INC)) result = 1; +#ifdef AUTO_INC_DEC + incdec_ok = true; +#endif break; case 'E': @@ -1814,6 +1823,23 @@ asm_operand_ok (rtx op, const char *cons return 0; } +#ifdef AUTO_INC_DEC + /* For operands without < or > constraints reject side-effects. */ + if (!incdec_ok && result && MEM_P (op)) + switch (GET_CODE (XEXP (op, 0))) + { + case PRE_INC: + case POST_INC: + case PRE_DEC: + case POST_DEC: + case PRE_MODIFY: + case POST_MODIFY: + return 0; + default: + break; + } +#endif + return result; } @@ -2039,6 +2065,7 @@ extract_insn (rtx insn) recog_data.n_operands = 0; recog_data.n_alternatives = 0; recog_data.n_dups = 0; + recog_data.is_asm = false; switch (GET_CODE (body)) { @@ -2085,6 +2112,7 @@ extract_insn (rtx insn) while (*p) recog_data.n_alternatives += (*p++ == ','); } + recog_data.is_asm = true; break; } fatal_insn_not_found (insn); @@ -2699,6 +2727,30 @@ constrain_operands (int strict) = recog_data.operand[funny_match[funny_match_index].this_op]; } +#ifdef AUTO_INC_DEC + /* For operands without < or > constraints reject side-effects. */ + if (recog_data.is_asm) + { + for (opno = 0; opno < recog_data.n_operands; opno++) + if (MEM_P (recog_data.operand[opno])) + switch (GET_CODE (XEXP (recog_data.operand[opno], 0))) + { + case PRE_INC: + case POST_INC: + case PRE_DEC: + case POST_DEC: + case PRE_MODIFY: + case POST_MODIFY: + if (strchr (recog_data.constraints[opno], '<') == NULL + || strchr (recog_data.constraints[opno], '>') + == NULL) + return 0; + break; + default: + break; + } + } +#endif return 1; } } --- gcc/doc/md.texi.jj 2010-06-14 17:14:16.000000000 +0200 +++ gcc/doc/md.texi 2010-06-20 16:28:51.199197446 +0200 @@ -1052,6 +1052,10 @@ an operand may be in a register, and whi operand can be a memory reference, and which kinds of address; whether the operand may be an immediate constant, and which possible values it may have. Constraints can also require two operands to match. +Side-effects aren't allowed in operands of inline @code{asm}, unless +@samp{<} or @samp{>} constraints are used, because there is no guarantee +that the side-effects will happen exactly once in an instruction that can update +the addressing register. @ifset INTERNALS @menu @@ -1129,12 +1133,21 @@ would fit the @samp{m} constraint but no @cindex @samp{<} in constraint @item @samp{<} A memory operand with autodecrement addressing (either predecrement or -postdecrement) is allowed. +postdecrement) is allowed. In inline @code{asm} this constraint is only +allowed if the operand is used exactly once in an instruction that can +handle the side-effects. Not using an operand with @samp{<} in constraint +string in the inline @code{asm} pattern at all or using it in multiple +instructions isn't valid, because the side-effects wouldn't be performed +or would be performed more than once. Furthermore, on some targets +the operand with @samp{<} in constraint string must be accompanied by +special instruction suffixes like @code{%U0} instruction suffix on PowerPC +or @code{%P0} on IA-64. @cindex @samp{>} in constraint @item @samp{>} A memory operand with autoincrement addressing (either preincrement or -postincrement) is allowed. +postincrement) is allowed. In inline @code{asm} the same restrictions +as for @samp{<} apply. @cindex @samp{r} in constraint @cindex registers in constraints --- gcc/testsuite/g++.dg/torture/pr44492.C.jj 2010-06-20 16:15:01.221230004 +0200 +++ gcc/testsuite/g++.dg/torture/pr44492.C 2010-06-20 16:15:01.221230004 +0200 @@ -0,0 +1,31 @@ +// PR middle-end/44492 +// { dg-do run } + +struct T { unsigned long p; }; +struct S { T a, b, c; unsigned d; }; + +__attribute__((noinline)) +void +bar (const T &x, const T &y) +{ + if (x.p != 0x2348 || y.p != 0x2346) + __builtin_abort (); +} + +__attribute__((noinline)) +void +foo (S &s, T e) +{ + unsigned long a = e.p; + unsigned long b = s.b.p; + __asm__ volatile ("" : : "rm" (a), "rm" (b)); + bar (e, s.b); +} + +int +main () +{ + S s = { { 0x2345 }, { 0x2346 }, { 0x2347 }, 6 }; + T t = { 0x2348 }; + foo (s, t); +}