From patchwork Fri Feb 12 07:01:17 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Wohlferd X-Patchwork-Id: 582102 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 3E6DF140BFC for ; Fri, 12 Feb 2016 18:02:07 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=tCUhmsDG; 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 :subject:to:references:cc:from:message-id:date:mime-version :in-reply-to:content-type; q=dns; s=default; b=dy1zRVeVlxdoMcJ9S hbBtGsUO+5US2zzGijtHkRy3BGTeaJC3BLqF8WjL4u5tA8XFSAMOjSD+sIMk0G7i xtM7GZjS12kMeLT2m8apqlbyuF6/uChuOSMDwsiOjAo9Ym0gWIGJSOfrOndVFxcn MeG3XF+fjPsX/NznKm9UIlwJ9w= 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 :subject:to:references:cc:from:message-id:date:mime-version :in-reply-to:content-type; s=default; bh=YHfuW572X2jB8NHyLE/lsmy 4Ixk=; b=tCUhmsDGSYy7zhvsxhKdQM4jePcpvg4fL1hMLwXjgx+l/d3feYs24ZL y4DzyLZMBonzEhlSNviT7I/QA1AnEhi5T4C1+IzAA98mbqN8ISWdXF2eb8YAfRoi fgdE50lTb7Ngn8jI3qmyk5aDugZMHA1mLFfith6hFuUEnsmmH1WA= Received: (qmail 17409 invoked by alias); 12 Feb 2016 07:01:57 -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 17396 invoked by uid 89); 12 Feb 2016 07:01:56 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.1 required=5.0 tests=AWL, BAYES_00, KAM_ASCII_DIVIDERS, RCVD_IN_DNSWL_NONE, RP_MATCHES_RCVD, SPF_PASS autolearn=no version=3.3.2 spammy=Comments, H*r:esmtpa X-HELO: bosmailout09.eigbox.net Received: from bosmailout09.eigbox.net (HELO bosmailout09.eigbox.net) (66.96.188.9) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Fri, 12 Feb 2016 07:01:54 +0000 Received: from bosmailscan07.eigbox.net ([10.20.15.7]) by bosmailout09.eigbox.net with esmtp (Exim) id 1aU7jk-0006OP-Ep for gcc-patches@gcc.gnu.org; Fri, 12 Feb 2016 02:01:52 -0500 Received: from [10.115.3.32] (helo=bosimpout12) by bosmailscan07.eigbox.net with esmtp (Exim) id 1aU7jk-0007sb-DS for gcc-patches@gcc.gnu.org; Fri, 12 Feb 2016 02:01:52 -0500 Received: from bosauthsmtp04.yourhostingaccount.com ([10.20.18.4]) by bosimpout12 with id HK1m1s00R05GATN01K1pvn; Fri, 12 Feb 2016 02:01:52 -0500 X-Authority-Analysis: v=2.1 cv=HK1NF+dv c=1 sm=1 tr=0 a=jcMFiYeD4Gz02xRQ37rMPA==:117 a=sZx1nW7oDdbgogxTPqu5Xw==:17 a=L9H7d07YOLsA:10 a=9cW_t1CCXrUA:10 a=s5jvgZ67dGcA:10 a=88b2x-oFWvEA:10 a=jFJIQSaiL_oA:10 a=r77TgQKjGQsHNAKrUKIA:9 a=C8F9KGFtAAAA:8 a=k1olXaS9RYVOtNkVNRwA:9 a=pILNOxqGKmIA:10 a=mDV3o1hIAAAA:8 a=NWxgZQlzxPPKUM69ZnEA:9 a=_zRMieAGw3AOGz-g:21 a=uO8C2tn-Ai8tqJaJ:21 Received: from [207.118.20.56] (port=50430 helo=[192.168.1.160]) by bosauthsmtp04.eigbox.net with esmtpa (Exim) id 1aU7je-0000dj-BN; Fri, 12 Feb 2016 02:01:46 -0500 Subject: Re: AW: AW: Wonly-top-basic-asm To: Bernd Edlinger , Bernd Schmidt , "gcc-patches@gcc.gnu.org" , Richard Henderson , "jason@redhat.com" References: <56A54EF9.8060006@LimeGreenSocks.com> <56A61442.3090803@redhat.com> <56A9C134.1030500@LimeGreenSocks.com> <56B80F57.9020606@LimeGreenSocks.com> <56BBCC90.9020001@LimeGreenSocks.com> Cc: "segher@kernel.crashing.org" , "sandra@codesourcery.com" , "Paul_Koning@Dell.com" , Jeff Law , "bernds_cb1@t-online.de" , Andrew Haley , David Wohlferd From: David Wohlferd Message-ID: <56BD833D.9060308@LimeGreenSocks.com> Date: Thu, 11 Feb 2016 23:01:17 -0800 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 MIME-Version: 1.0 In-Reply-To: X-EN-UserInfo: 97390230d6758ac7ebdf93f8c6197d31:931c98230c6409dcc37fa7e93b490c27 X-EN-AuthUser: dw@limegreensocks.com X-EN-OrigIP: 207.118.20.56 X-EN-OrigHost: unknown > why not simply -Wbasic-asm ? Since both you and Bernd favor this shorter name, I have changed it. > Indentation wrong here. The whole block must be indented by 2 spaces. Fixed. > Comments should end with dot space space */ Fixed. > the DECL_ATTRIBUTES should be at the same column as the "naked". Fixed. > Comments should end with dot space space */ Fixed. > the DECL_ATTRIBUTES should be at the same column as the "naked". Fixed. > C++, isn't it always upper case? Fixed. > ChangeLog lines begin with TAB. Hmm. Thunderbird changed them to spaces. I've tried something different this time. Hopefully fixed. > Please split the ChangeLog > use relative file names. Fixed (I think). > Please add the function name where you changed in brackets. Fixed. ================================================== ChangeLog: 2016-02-11 David Wohlferd * doc/extend.texi: Doc basic asm behavior and new -Wbasic-asm option. * doc/invoke.texi: Doc new -Wbasic-asm option. ChangeLog: 2016-02-11 David Wohlferd * c.opt: Define -Wbasic-asm. ChangeLog: 2016-02-11 David Wohlferd * c-parser.c (c_parser_asm_statement): Implement -Wbasic-asm for C. ChangeLog: 2016-02-11 David Wohlferd * parser.c (cp_parser_asm_definition): Implement -Wbasic-asm for C++. ChangeLog: 2016-02-11 David Wohlferd * c-c++-common/Wbasic-asm.c: New tests for -Wbasic-asm. * c-c++-common/Wbasic-asm-2.c: Ditto. New patch is attached. Note that Bernd Schmidt and I are still discussing changes to the docs (see next message). dw Index: gcc/c-family/c.opt =================================================================== --- gcc/c-family/c.opt (revision 233367) +++ gcc/c-family/c.opt (working copy) @@ -585,6 +585,10 @@ C++ ObjC++ Var(warn_namespaces) Warning Warn on namespace definition. +Wbasic-asm +C ObjC ObjC++ C++ Var(warn_basic_asm) Warning +Warn on unsafe uses of basic asm. + Wsized-deallocation C++ ObjC++ Var(warn_sized_deallocation) Warning EnabledBy(Wextra) Warn about missing sized deallocation functions. Index: gcc/c/c-parser.c =================================================================== --- gcc/c/c-parser.c (revision 233367) +++ gcc/c/c-parser.c (working copy) @@ -5978,8 +5978,19 @@ labels = NULL_TREE; if (c_parser_next_token_is (parser, CPP_CLOSE_PAREN) && !is_goto) - goto done_asm; + { + /* Warn on basic asm used inside of functions, + except when in naked functions. Also allow asm (""). */ + if (warn_basic_asm && TREE_STRING_LENGTH (str) != 1) + if (lookup_attribute ("naked", + DECL_ATTRIBUTES (current_function_decl)) + == NULL_TREE) + warning_at (asm_loc, OPT_Wbasic_asm, + "asm statement in function does not use extended syntax"); + goto done_asm; + } + /* Parse each colon-delimited section of operands. */ nsections = 3 + is_goto; for (section = 0; section < nsections; ++section) Index: gcc/cp/parser.c =================================================================== --- gcc/cp/parser.c (revision 233367) +++ gcc/cp/parser.c (working copy) @@ -18041,6 +18041,8 @@ bool goto_p = false; required_token missing = RT_NONE; + location_t asm_loc = cp_lexer_peek_token (parser->lexer)->location; + /* Look for the `asm' keyword. */ cp_parser_require_keyword (parser, RID_ASM, RT_ASM); @@ -18199,6 +18201,17 @@ /* If the extended syntax was not used, mark the ASM_EXPR. */ if (!extended_p) { + /* Warn on basic asm used inside of functions, + EXCEPT when in naked functions. Also allow asm (""). */ + if (warn_basic_asm + && TREE_STRING_LENGTH (string) != 1) + if (lookup_attribute ("naked", + DECL_ATTRIBUTES (current_function_decl)) + == NULL_TREE) + warning_at (asm_loc, OPT_Wbasic_asm, + "asm statement in function does not use extended" + " syntax"); + tree temp = asm_stmt; if (TREE_CODE (temp) == CLEANUP_POINT_EXPR) temp = TREE_OPERAND (temp, 0); Index: gcc/doc/extend.texi =================================================================== --- gcc/doc/extend.texi (revision 233367) +++ gcc/doc/extend.texi (working copy) @@ -7458,7 +7458,8 @@ @end table @subsubheading Remarks -Using extended @code{asm} typically produces smaller, safer, and more +Using extended @code{asm} (@pxref{Extended Asm}) typically produces smaller, +safer, and more efficient code, and in most cases it is a better solution than basic @code{asm}. However, there are two situations where only basic @code{asm} can be used: @@ -7516,11 +7517,51 @@ Basic @code{asm} provides no mechanism to provide different assembler strings for different dialects. -Here is an example of basic @code{asm} for i386: +Basic @code{asm} statements do not perform an implicit "memory" clobber +(@pxref{Clobbers}). Also, there is no implicit clobbering of @emph{any} +registers, so (other than in @code{naked} functions which follow the ABI +rules) changed registers must be restored to their original value before +exiting the @code{asm}. While this behavior has not always been +documented, GCC has worked this way since at least v2.95.3. +@strong{Warning:} This "clobber nothing" behavior may be different than how +other compilers treat basic @code{asm}, since the C standards for the +@code{asm} statement provide no guidance regarding these semantics. As a +result, @code{asm} statements that work correctly on other compilers may not +work correctly with GCC (and vice versa), even though they both compile +without error. + +Future versions of GCC may change basic @code{asm} to clobber memory and +perhaps some (or all) registers. This change may fix subtle problems with +existing @code{asm} statements. However it may break or slow down ones +that were working correctly. To ``future-proof'' your asm against possible +changes to basic @code{asm}'s semantics, use extended @code{asm}. + +You can use @option{-Wbasic-asm} to locate basic @code{asm} +statements that may need changes, and refer to +@uref{https://gcc.gnu.org/wiki/ConvertBasicAsmToExtended, How to convert +from basic asm to extended asm} for information about how to perform the +conversion. + +Here is an example of top-level basic @code{asm} for i386 that defines an +asm macro. That macro is then invoked from within a function using +extended @code{asm}: + @example -/* Note that this code will not compile with -masm=intel */ -#define DebugBreak() asm("int $3") +/* Define macro at file scope with basic asm. */ +/* Add macro parameter p to eax. */ +asm (".macro test p\n\t" + "addl $\\p, %eax\n\t" + ".endm"); + +/* Use macro in function using extended asm. It needs + the "cc" clobber since the flags are changed and uses + the "a" constraint since it modifies eax. */ +int DoAdd (int value) +@{ + asm ("test 5" : "+a" (value) : : "cc"); + return value; +@} @end example @node Extended Asm Index: gcc/doc/invoke.texi =================================================================== --- gcc/doc/invoke.texi (revision 233367) +++ gcc/doc/invoke.texi (working copy) @@ -5727,6 +5727,21 @@ a structure that has been marked with the @code{designated_init} attribute. +@item -Wbasic-asm @r{(C and C++ only)} +Warn if basic @code{asm} statements are used inside a function (i.e. not at +top-level/file scope). + +When used inside of functions, basic @code{asm} can result in unexpected and +unwanted variations in behavior between compilers due to how registers are +handled when calling the asm (@pxref{Basic Asm}). The lack of input and +output constraints (@pxref{Extended Asm}) can also make it difficult for +optimizers to correctly and consistently position the output relative to +other code. + +Functions that are marked with the @option{naked} attribute (@pxref{Function +Attributes}) and @code{asm} statements with an empty instruction string are +excluded from this check. + @item -Whsa Issue a warning when HSAIL cannot be emitted for the compiled function or OpenMP construct. Index: gcc/testsuite/c-c++-common/Wbasic-asm-2.c =================================================================== --- gcc/testsuite/c-c++-common/Wbasic-asm-2.c (revision 0) +++ gcc/testsuite/c-c++-common/Wbasic-asm-2.c (working copy) @@ -0,0 +1,15 @@ +/* { dg-do compile } */ +/* { dg-require-effective-target naked_functions } */ +/* { dg-options "-Wbasic-asm" } */ + +int __attribute__((naked)) +func (int x, int y) +{ + /* Basic asm should not warn in naked functions. */ + asm (" "); /* No warning. */ +} + +int main (int argc, char *argv[]) +{ + return func (argc, argc); +} Index: gcc/testsuite/c-c++-common/Wbasic-asm.c =================================================================== --- gcc/testsuite/c-c++-common/Wbasic-asm.c (revision 0) +++ gcc/testsuite/c-c++-common/Wbasic-asm.c (working copy) @@ -0,0 +1,43 @@ +/* { dg-do compile } */ +/* { dg-options "-Wbasic-asm" } */ + +#if defined (__i386__) || defined (__x86_64__) +/* Acceptable. */ +register int b asm ("esi"); +#else +int b = 3; +#endif + +/* Acceptable. */ +int foo asm ("myfoo") = 2; + +/* Acceptable. */ +asm (" "); + +/* Acceptable. */ +int func (int x, int y) asm ("MYFUNC"); + +int main (int argc, char *argv[]) +{ +#if defined (__i386__) || defined (__x86_64__) + /* Acceptable. */ + register int a asm ("edi"); +#else + int a = 2; +#endif + + /* Acceptable. */ + asm (" "::"r"(a), "r" (b)); + + /* Acceptable. */ + asm goto (""::::done); + + /* Acceptable. */ + asm (""); + + /* Acceptable. */ + asm (" "); /* { dg-warning "does not use extended syntax" } */ + + done: + return 0; +}