| Message ID | 56C7BB50.4000909@LimeGreenSocks.com |
|---|---|
| State | New |
| Headers | show |
On 20.02.2016 02:03, David Wohlferd wrote: > @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 testme 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 ("testme 5" : "+a" (value) : : "cc"); > + return value; > +@} > @end example Sorry, but I don't like this example at all. First the new example is essentially academic and useless, while the previous example could well be used in real world code, except we could note here that we also have a __builtin_trap () now. Second if you don't mention that "cc" is already implicitly clobbered by default, and if it is written here it is ignored on i386 targets, then everybody will hurry to modify their asm code when there is no real reason for that. Bernd.
On 2/20/2016 4:08 AM, Bernd Edlinger wrote: > Sorry, but I don't like this example at all. > > First the new example is essentially academic and useless, When used within a function, basic asm: - causes difficulties for optimizers - produces incompatibilities with other compilers - has semantics that are the opposite of what users expect - may soon produce incompatibilities with other versions of gcc Given all this, why would we want our sample to show using basic asm within a function (as you are suggesting) instead of working to discourage it? Contrawise, even people who are advocating for the deprecation/removal of basic asm (like me) acknowledge that it is needed for top level. That is why I use it for the sample. I suppose I could grab some actual top level code from the linux source (like ".symver __netf2_compat,__netf2@GCC_3.0"). But while it's real-world instead of "academic," it's also not as clear as I'd like for a sample. Obviously this testme code isn't going to be placed verbatim in someone's project. However, if someone wants to use asm macros (a plausible if uncommon case), this sample shows them how, using a simple, easy-to-understand (if not particularly useful) method. In contrast, the "int $3" you seem to favor doesn't really show anything (even how to do multiple lines). And worse, Jeff's plan is to change basic asm statements so they clobber ALL registers plus memory. Even a trivial example shows how this results in gcc generating completely different code around an "int $3." Which makes it rather problematical for debugging (can you say heisenbug?), which is the primary use for int 3. > while the previous example could well be used > in real world code, except we could note here > that we also have a __builtin_trap () now. After reading the documentation for __builtin_trap, I still have absolutely no idea what it does. Worse, after NOT saying precisely what it does on any given platform, the docs make it clear that whatever it does isn't fixed and can change at any time. I'm a big believer in using builtins instead of inline asm. But based on these docs, I can't ever see using __builtin_trap myself. > Second if you don't mention that "cc" is already implicitly > clobbered by default, and if it is written here it is ignored > on i386 targets, then everybody will hurry to modify their > asm code when there is no real reason for that. The meaning of the "cc" clobber is defined under extended asm. The usage in this sample is consistent with that definition. In the (unlikely) event that "everybody" decides to change their code, they will all have (slightly) better documented code which is written in accordance with the docs. And which behaves precisely the same as what they have now. So now what? I have one Bernd who likes the sample, and one who doesn't. Obviously I think what I'm proposing is better than what's there now and I've done my best to say why. But me believing it to be better doesn't get anything checked in. What will? dw
On 02/21/2016 11:27 AM, David Wohlferd wrote: > So now what? I have one Bernd who likes the sample, and one who > doesn't. Obviously I think what I'm proposing is better than what's > there now and I've done my best to say why. But me believing it to be > better doesn't get anything checked in. I hadn't thought it through well enough. Jan's objection (order isn't guaranteed) is relevant. I'd drop the example. Bernd
On 2/26/2016 7:09 AM, Bernd Schmidt wrote: > On 02/21/2016 11:27 AM, David Wohlferd wrote: >> So now what? I have one Bernd who likes the sample, and one who >> doesn't. Obviously I think what I'm proposing is better than what's >> there now and I've done my best to say why. But me believing it to be >> better doesn't get anything checked in. > > I hadn't thought it through well enough. Jan's objection (order isn't > guaranteed) is relevant. I'd drop the example. To be clear: Are you suggesting that we delete the sample that is there and have no example at all for basic asm? I'm not sure I agree. Looking at the linux kernel source, there are times and places where basic asm is appropriate, even necessary. I realize that macros are an uncommon usage. But it makes for a more interesting sample than simply outputting a section name. If ordering is your concern, would adding a reference to -fno-toplevel-reorder make you feel better about this? It seems unnecessary in this particular context, but mentioning this option on the basic asm page is certainly appropriate. dw
So, we have been discussing this issue for 4 months now. Over that time, I have tried to incorporate everyone's feedback. As a result we have gone from a tiny doc patch (just describe the current semantics), to a big doc patch (completely deprecate basic asm when used in a function) to a medium doc patch + code fix (warning when using basic asm in a function) and now back to a slightly-bigger-than-tiny doc patch. I have made no changes since the last patch I posted (https://gcc.gnu.org/ml/gcc-patches/2016-02/msg01406.html) for the reasons discussed below. I assert that this patch both contains important information users need and is better than the current text. I expect that Sandra is prepared to check this in as soon as someone signs off on its technical accuracy. dw On 2/28/2016 11:02 PM, David Wohlferd wrote: > On 2/26/2016 7:09 AM, Bernd Schmidt wrote: >> On 02/21/2016 11:27 AM, David Wohlferd wrote: >>> So now what? I have one Bernd who likes the sample, and one who >>> doesn't. Obviously I think what I'm proposing is better than what's >>> there now and I've done my best to say why. But me believing it to be >>> better doesn't get anything checked in. >> >> I hadn't thought it through well enough. Jan's objection (order isn't >> guaranteed) is relevant. I'd drop the example. > > To be clear: Are you suggesting that we delete the sample that is > there and have no example at all for basic asm? > > I'm not sure I agree. Looking at the linux kernel source, there are > times and places where basic asm is appropriate, even necessary. I > realize that macros are an uncommon usage. But it makes for a more > interesting sample than simply outputting a section name. > > If ordering is your concern, would adding a reference to > -fno-toplevel-reorder make you feel better about this? It seems > unnecessary in this particular context, but mentioning this option on > the basic asm page is certainly appropriate. > > dw >
On 03/11/2016 01:55 AM, David Wohlferd wrote: > So, we have been discussing this issue for 4 months now. Over that > time, I have tried to incorporate everyone's feedback. > > As a result we have gone from a tiny doc patch (just describe the > current semantics), to a big doc patch (completely deprecate basic asm > when used in a function) to a medium doc patch + code fix (warning when > using basic asm in a function) and now back to a > slightly-bigger-than-tiny doc patch. > > I have made no changes since the last patch I posted > (https://gcc.gnu.org/ml/gcc-patches/2016-02/msg01406.html) for the > reasons discussed below. > > I assert that this patch both contains important information users need > and is better than the current text. I expect that Sandra is prepared > to check this in as soon as someone signs off on its technical accuracy. The example is not good, as discussed previously, and IMO the best option is to remove it. Otherwise I have no objections to the latest variant. Bernd
Index: extend.texi =================================================================== --- extend.texi (revision 233367) +++ 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: @@ -7498,10 +7499,25 @@ symbol errors during compilation if your assembly code defines symbols or labels. -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. +@strong{Warning:} The C standards do not specify semantics for @code{asm}, +making it a potential source of incompatibilities between compilers. These +incompatibilities may not produce compiler warnings/errors. +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, consider replacing basic @code{asm} +with extended @code{asm}. See +@uref{https://gcc.gnu.org/wiki/ConvertBasicAsmToExtended, How to convert +from basic asm to extended asm} for information about how to perform this +conversion. + The compiler copies the assembler instructions in a basic @code{asm} verbatim to the assembly language output file, without processing dialects or any of the @samp{%} operators that are available with @@ -7516,11 +7532,25 @@ Basic @code{asm} provides no mechanism to provide different assembler strings for different dialects. -Here is an example of basic @code{asm} for i386: +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 testme 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 ("testme 5" : "+a" (value) : : "cc"); + return value; +@} @end example @node Extended Asm
On 2/13/2016 8:00 PM, David Wohlferd wrote: > Fair enough. Committing what we can right now sounds like a good plan. Attached is the doc patch, minus the proposed warning. ChangeLog: 2016-02-19 David Wohlferd <dw@LimeGreenSocks.com> Bernd Schmidt <bschmidt@redhat.com> * doc/extend.texi: Doc basic asm behavior re clobbers. dw