| Message ID | 56BBCC90.9020001@LimeGreenSocks.com |
|---|---|
| State | New |
| Headers | show |
On 11.2.2016, David Wohlferd wrote: > > Since no one expressed any objections, I have renamed the option from > -Wonly-top-basic-asm to -Wbasic-asm-in-function. This more clearly > conveys what the option does (give a warning if you find basic asm in a > function). > why not simply -Wbasic-asm ? > I believe the attached patch addresses all the other outstanding comments. > Indentation wrong here. The whole block must be indented by 2 spaces. > if (c_parser_next_token_is (parser, CPP_CLOSE_PAREN) && !is_goto) >+ { >+ /* Warn on basic asm used inside of functions, >+ EXCEPT when in naked functions. Also allow asm (""). */ Comments should end with dot space space */ >+ if (warn_basic_asm_in_function && TREE_STRING_LENGTH (str) != 1) >+ if (lookup_attribute ("naked", >+ DECL_ATTRIBUTES (current_function_decl)) the DECL_ATTRIBUTES should be at the same column as the "naked". >+ == NULL_TREE) >+ warning_at (asm_loc, OPT_Wbasic_asm_in_function, >+ "asm statement in function does not use extended syntax"); >+ > goto done_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 (""). */ Comments should end with dot space space */ >+ if (warn_basic_asm_in_function >+ && TREE_STRING_LENGTH (string) != 1) >+ if (lookup_attribute ("naked", >+ DECL_ATTRIBUTES (current_function_decl)) the DECL_ATTRIBUTES should be at the same column as the "naked". > ChangeLog: > 2016-02-10 David Wohlferd <dw@LimeGreenSocks.com> > > * doc/extend.texi: Doc basic asm behavior and new > -Wbasic-asm-in-function option. > * doc/invoke.texi: Doc new -Wbasic-asm-in-function option. > * c-family/c.opt: Define -Wbasic-asm-in-function. > * c/c-parser.c: Implement -Wbasic-asm-in-function for C. > * cp/parser.c: Implement -Wbasic-asm-in-function for c++. C++, isn't it always upper case? > * testsuite/c-c++-common/Wbasic-asm-in-function.c: New tests for > -Wbasic-asm-in-function. > * testsuite/c-c++-common/Wbasic-asm-in-function-2.c: Ditto. > ChangeLog lines begin with TAB. Please split the ChangeLog, there are separate ChangeLogs at gcc/ChangeLog (doc changes go in there) gcc/c/ChangeLog, gcc/cp/ChangeLog, gcc/c-family/ChangLog and gcc/testsuite/ChangeLog, the respective ChangeLog entries use relative file names. Please add the function name where you changed in brackets. For instance: * c-parser.c (cp_parser_asm_definition): Implement -Wbasic-asm-in-function. Thanks Bernd. > While I have a release on file with FSF, I don't have write access to SVN. > > dw
On 02/11/2016 12:49 AM, David Wohlferd wrote: > I believe the attached patch addresses all the other outstanding comments. Bernd Edlinger made some thorough comments; I'll just add a few more. I don't think this is a patch we're considering for gcc-6, at least not for the initial release - I imagine it could be backported from gcc-7 at some point. Like the other Bernd I have a preference for just -Wbasic-asm. I'd make the analogy with -Wparentheses - we don't warn about every parenthesis, but the name of an option is not the place for detailed explanations of how it works. Less typing and less to remember is preferrable IMO. > + /* Warn on basic asm used inside of functions, > + EXCEPT when in naked functions. Also allow asm (""). */ No all-caps. > @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 Rewrap the paragraph? > -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-in-function} 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. I still think this is too verbose and would serve to confuse rather than enlighten in practice. (If anyone feels otherwise, feel free to overrule me.) I'm also no longer sure about references to the wiki. Let's look at this existing paragraph from the manual: Since GCC does not parse the @var{AssemblerInstructions}, it has no visibility of any symbols it references. This may result in GCC discarding those symbols as unreferenced. I think extending this would be better. Something like "Since the C standards does not specify semantics for @code{asm}, it is a potential source of incompatibilities between compilers. GCC does not parse the @var{AssemblerInstructions}, which means there is no way to communicate to the compiler what is happening inside them. GCC has no visibility of any symbols referenced in the @code{asm} and may discard them as unreferenced. It also does not know about side effects that may occur, such as modifications of memory locations or registers. GCC assumes that no such side effects occur, which may not be what the user expected if code was written for other compilers. Since basic @code{asm} cannot easily be used in a reliable way, @option{-Wbasic-asm} should be used to warn about the use of basic asm inside a function. See @uref{https://gcc.gnu.org/wiki/ConvertBasicAsmToExtended, How to convert from basic asm to extended asm} for information about how to convert code to use extended @code{asm}." But again, if someone feels the docs patch as posted is preferrable, go ahead and approve it (for stage1 I assume). Bernd
On 02/11/2016 08:40 AM, Bernd Schmidt wrote: > But again, if someone feels the docs patch as posted is preferrable, go > ahead and approve it (for stage1 I assume). TBH, I haven't looked at the documentation patch at all; I've been ignoring this issue because (a) I thought the technical details were still under discussion and (b) adding a new command-line option is stage 1 material not suitable to commit right now anyway. I'll take a look at the docs once the name and behavior of the new option have been settled upon. -Sandra
> I don't think this is a patch we're considering for gcc-6, at least > not for the initial release - I imagine it could be backported from > gcc-7 at some point. Actually, it was my intent that this apply to v6. It's not like there is a significant change here. We're documenting long-time behavior, and adding a (disabled) warning. The reasons I think this is needed for v6 are: 1) We have become aware of places where basic asm's existing behavior is a problem. This patch provides a way for users to locate these issues with a minimal, non-intrusive change. 2) There is a significant change to this behavior being proposed for v7. When this happens, having a way to locate affected statements with features from a stable release seems desirable. > Like the other Bernd I have a preference for just -Wbasic-asm. I'd > make the analogy with -Wparentheses - we don't warn about every > parenthesis, but the name of an option is not the place for detailed > explanations of how it works. Less typing and less to remember is > preferrable IMO. While I still prefer Wbasic-asm-in-function, I really don't have strong feelings here. Since both you and Bernd prefer this, I have changed it. >> + /* Warn on basic asm used inside of functions, >> + EXCEPT when in naked functions. Also allow asm (""). */ > > No all-caps. Fixed. >> @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 > > Rewrap the paragraph? I could, but then people get cranky about how hard the patch is to review. >> -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-in-function} 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. > > I still think this is too verbose and would serve to confuse rather > than enlighten in practice. (If anyone feels otherwise, feel free to > overrule me.) I assume you mean someone besides me... > I'm also no longer sure about references to the wiki. This is not the first reference to the gcc wiki in the docs (looks like the 6th for gcc, plus 10 for other wikis). > Let's look at this existing paragraph from the manual: > > Since GCC does not parse the @var{AssemblerInstructions}, it has > no visibility of any symbols it references. This may result in GCC > discarding those symbols as unreferenced. > > I think extending this would be better. Something like > > "Since the C standards does not specify semantics for @code{asm}, it > is a potential source of incompatibilities between compilers. GCC does > not parse the @var{AssemblerInstructions}, which means there is no way > to communicate to the compiler what is happening inside them. GCC has > no visibility of any symbols referenced in the @code{asm} and may > discard them as unreferenced. It also does not know about side effects > that may occur, such as modifications of memory locations or > registers. GCC assumes that no such side effects occur, which may not > be what the user expected if code was written for other compilers. > > Since basic @code{asm} cannot easily be used in a reliable way, > @option{-Wbasic-asm} should be used to warn about the use of basic asm > inside a function. See > @uref{https://gcc.gnu.org/wiki/ConvertBasicAsmToExtended, How to > convert from basic asm to extended asm} for information about how to > convert code to use extended @code{asm}." Hmm. Yes, that's better. But there are some things that got lost here that I think are important. How about: ------------ @strong{Warning:} The C standards do not specify semantics for @code{asm}, making it a potential source of incompatibilities between compilers. @code{asm} statements that work correctly on other compilers may not work correctly with GCC (and vice versa), even though both compile without error. GCC does not parse basic @code{asm}'s @var{AssemblerInstructions}, which means there is no way to communicate to the compiler what is happening inside them. GCC has no visibility of symbols in the @code{asm} and may discard them as unreferenced. It also does not know about side effects of the assembler code, such as modifications to memory or registers. Unlike some compilers, GCC assumes that no changes to either memory or registers occur. This assumption may change in a future release. To avoid complications from future changes to the semantics and the compatibility issues between compilers, use @option{-Wbasic-asm} to warn about the use of basic asm inside a function. See @uref{https://gcc.gnu.org/wiki/ConvertBasicAsmToExtended, How to convert from basic asm to extended asm} for information about how to convert code to use extended @code{asm}. ------------ dw
On 2/11/2016 8:03 AM, Sandra Loosemore wrote: > On 02/11/2016 08:40 AM, Bernd Schmidt wrote: > >> But again, if someone feels the docs patch as posted is preferrable, go >> ahead and approve it (for stage1 I assume). > > TBH, I haven't looked at the documentation patch at all; I've been > ignoring this issue because (a) I thought the technical details were > still under discussion and (b) adding a new command-line option is > stage 1 material not suitable to commit right now anyway. > > I'll take a look at the docs once the name and behavior of the new > option have been settled upon. So far, no one has suggested any changes to the behavior. And we seem to have settled on a name. dw
On 02/12/2016 08:05 AM, David Wohlferd wrote: > Actually, it was my intent that this apply to v6. It's not like there > is a significant change here. We're documenting long-time behavior, and > adding a (disabled) warning. The doc patch (minus mentioning the warning) could go in now, but for gcc-6 we're at a stage where we're only accepting regression fixes with very few exceptions. If you can convince a RM that this is important enough then it could still go in. > 2) There is a significant change to this behavior being proposed for > v7. When this happens, having a way to locate affected statements with > features from a stable release seems desirable. I'm actually not convinced that we'll want to change much in asm behaviour. Clobbering memory, maybe, but I can't see much beyond that - there's just no gain and some risk. So I'm a little more relaxed about the whole thing. >> "Since the C standards does not specify semantics for @code{asm}, it >> is a potential source of incompatibilities between compilers. GCC does >> not parse the @var{AssemblerInstructions}, which means there is no way >> to communicate to the compiler what is happening inside them. GCC has >> no visibility of any symbols referenced in the @code{asm} and may >> discard them as unreferenced. It also does not know about side effects >> that may occur, such as modifications of memory locations or >> registers. GCC assumes that no such side effects occur, which may not >> be what the user expected if code was written for other compilers. >> >> Since basic @code{asm} cannot easily be used in a reliable way, >> @option{-Wbasic-asm} should be used to warn about the use of basic asm >> inside a function. See >> @uref{https://gcc.gnu.org/wiki/ConvertBasicAsmToExtended, How to >> convert from basic asm to extended asm} for information about how to >> convert code to use extended @code{asm}." > > Hmm. Yes, that's better. But there are some things that got lost here > that I think are important. How about: > > ------------ > @strong{Warning:} The C standards do not specify semantics for > @code{asm}, making it a potential source of incompatibilities between > compilers. @code{asm} statements that work correctly on other compilers > may not work correctly with GCC (and vice versa), even though both > compile without error. This is what I mean when I say "too verbose" - the second sentence essentially says exactly the same thing as the first. The repetition is unnecessary, and I'd drop it. > GCC does not parse basic @code{asm}'s @var{AssemblerInstructions}, which > means there is no way to communicate to the compiler what is happening > inside them. GCC has no visibility of symbols in the @code{asm} and may > discard them as unreferenced. It also does not know about side effects > of the assembler code, such as modifications to memory or registers. > Unlike some compilers, GCC assumes that no changes to either memory or > registers occur. This assumption may change in a future release. > > To avoid complications from future changes to the semantics and the > compatibility issues between compilers, use @option{-Wbasic-asm} to warn > about the use of basic asm inside a function. See > @uref{https://gcc.gnu.org/wiki/ConvertBasicAsmToExtended, How to convert > from basic asm to extended asm} for information about how to convert > code to use extended @code{asm}. Other than that they look quite similar, and I think your new suggestion is good too. Let's let Sandra have the last word. Bernd
On 02/12/2016 05:51 AM, Bernd Schmidt wrote: > On 02/12/2016 08:05 AM, David Wohlferd wrote: >> Actually, it was my intent that this apply to v6. It's not like there >> is a significant change here. We're documenting long-time behavior, and >> adding a (disabled) warning. > > The doc patch (minus mentioning the warning) could go in now, but for > gcc-6 we're at a stage where we're only accepting regression fixes with > very few exceptions. If you can convince a RM that this is important > enough then it could still go in. I looked at the last version of the patch I saw and this is my conclusion as well. If you would like me to commit just the doc change (minus the references to the new warning) now, please split the patch and I will do that. But, I cannot commit the change to add the new warning during Stage 4 without approval from a RM. -Sandra
On 2/12/2016 4:51 AM, Bernd Schmidt wrote: > On 02/12/2016 08:05 AM, David Wohlferd wrote: >> Actually, it was my intent that this apply to v6. It's not like there >> is a significant change here. We're documenting long-time behavior, and >> adding a (disabled) warning. > > The doc patch (minus mentioning the warning) could go in now, but for > gcc-6 we're at a stage where we're only accepting regression fixes > with very few exceptions. If you can convince a RM that this is > important enough then it could still go in. Understood. While I think the patch is small enough to be safe, whether it's important enough for this stage is a different question. >> 2) There is a significant change to this behavior being proposed for >> v7. When this happens, having a way to locate affected statements with >> features from a stable release seems desirable. > > I'm actually not convinced that we'll want to change much in asm > behaviour. Clobbering memory, maybe, but I can't see much beyond that > - there's just no gain and some risk. So I'm a little more relaxed > about the whole thing. And that's the question. If you are correct that we won't be changing this, then yeah, update the docs for v6, push the code change to v7. Done. But Jeff sounded quite serious when he said "the only rational behaviour for a traditional asm is that it has to be considered a use/clobber of memory and hard registers." If indeed that is the plan for v7, then having this warning available in v6 to allow people to prepare becomes important. Jeff Law: If you are listening, care to share your plans here? > Let's let Sandra have the last word [about the docs]. Good plan. dw
On 2/12/2016 5:03 PM, Sandra Loosemore wrote: > On 02/12/2016 05:51 AM, Bernd Schmidt wrote: >> On 02/12/2016 08:05 AM, David Wohlferd wrote: >>> Actually, it was my intent that this apply to v6. It's not like there >>> is a significant change here. We're documenting long-time behavior, >>> and >>> adding a (disabled) warning. >> >> The doc patch (minus mentioning the warning) could go in now, but for >> gcc-6 we're at a stage where we're only accepting regression fixes with >> very few exceptions. If you can convince a RM that this is important >> enough then it could still go in. > > I looked at the last version of the patch I saw and this is my > conclusion as well. If you would like me to commit just the doc > change (minus the references to the new warning) now, please split the > patch and I will do that. But, I cannot commit the change to add the > new warning during Stage 4 without approval from a RM. Fair enough. Committing what we can right now sounds like a good plan. Bernd and I have both posted alternate text to what was in the last patch (see https://gcc.gnu.org/ml/gcc-patches/2016-02/msg00861.html). He proposed (and I agreed) that having you make the call about which was better might be reasonable way to finalize this. If you want to pick one, I'll remove the Wbasic-asm and turn it into a doc-only patch. Or maybe you'd rather scrap them both and propose your own? I'm flexible here. There are important concepts that need to be conveyed. Doesn't much matter to me who writes them. dw
Index: gcc/c-family/c.opt =================================================================== --- gcc/c-family/c.opt (revision 233308) +++ gcc/c-family/c.opt (working copy) @@ -585,6 +585,10 @@ C++ ObjC++ Var(warn_namespaces) Warning Warn on namespace definition. +Wbasic-asm-in-function +C ObjC ObjC++ C++ Var(warn_basic_asm_in_function) 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 233308) +++ gcc/c/c-parser.c (working copy) @@ -5972,7 +5972,18 @@ labels = NULL_TREE; if (c_parser_next_token_is (parser, CPP_CLOSE_PAREN) && !is_goto) + { + /* Warn on basic asm used inside of functions, + EXCEPT when in naked functions. Also allow asm (""). */ + if (warn_basic_asm_in_function && TREE_STRING_LENGTH (str) != 1) + if (lookup_attribute ("naked", + DECL_ATTRIBUTES (current_function_decl)) + == NULL_TREE) + warning_at (asm_loc, OPT_Wbasic_asm_in_function, + "asm statement in function does not use extended syntax"); + goto done_asm; + } /* Parse each colon-delimited section of operands. */ nsections = 3 + is_goto; Index: gcc/cp/parser.c =================================================================== --- gcc/cp/parser.c (revision 233308) +++ 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_in_function + && TREE_STRING_LENGTH (string) != 1) + if (lookup_attribute ("naked", + DECL_ATTRIBUTES (current_function_decl)) + == NULL_TREE) + warning_at (asm_loc, OPT_Wbasic_asm_in_function, + "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 233308) +++ 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-in-function} 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 233308) +++ 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-in-function @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-in-function-2.c =================================================================== --- gcc/testsuite/c-c++-common/Wbasic-asm-in-function-2.c (revision 0) +++ gcc/testsuite/c-c++-common/Wbasic-asm-in-function-2.c (working copy) @@ -0,0 +1,15 @@ +/* { dg-do compile } */ +/* { dg-require-effective-target naked_functions } */ +/* { dg-options "-Wbasic-asm-in-function" } */ + +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-in-function.c =================================================================== --- gcc/testsuite/c-c++-common/Wbasic-asm-in-function.c (revision 0) +++ gcc/testsuite/c-c++-common/Wbasic-asm-in-function.c (working copy) @@ -0,0 +1,43 @@ +/* { dg-do compile } */ +/* { dg-options "-Wbasic-asm-in-function" } */ + +#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; +}
Since no one expressed any objections, I have renamed the option from -Wonly-top-basic-asm to -Wbasic-asm-in-function. This more clearly conveys what the option does (give a warning if you find basic asm in a function). I believe the attached patch addresses all the other outstanding comments. ChangeLog: 2016-02-10 David Wohlferd <dw@LimeGreenSocks.com> * doc/extend.texi: Doc basic asm behavior and new -Wbasic-asm-in-function option. * doc/invoke.texi: Doc new -Wbasic-asm-in-function option. * c-family/c.opt: Define -Wbasic-asm-in-function. * c/c-parser.c: Implement -Wbasic-asm-in-function for C. * cp/parser.c: Implement -Wbasic-asm-in-function for c++. * testsuite/c-c++-common/Wbasic-asm-in-function.c: New tests for -Wbasic-asm-in-function. * testsuite/c-c++-common/Wbasic-asm-in-function-2.c: Ditto. While I have a release on file with FSF, I don't have write access to SVN. dw