This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [DOC PATCH] Rewrite docs for inline asm


[ Oh no!  I wrote this on a plane two days ago and now it got stuck 
  in my outbox.  Let's get this patch in before any further delays
  happen. ]

On Sun, 27 Apr 2014, dw wrote:
> The goal of this patch was to rewrite section 6.41.  That's no simple 
> task, since 6.41 was ~10 very full pages.  Unfortunately the current 
> text is so random, I couldn't think of any way to update it piecemeal.

Understood.  Let's see that we can get an update committed soon.
We can always improve on it further later on, which then will be
a lot easier to do, review, and get pushed.

> A number of your concerns seem to stem from reviewing this text only by 
> looking at the patch.  I think your questions might answered by looking 
> at the actual generated output.

Right, I just did not want to delay this any further.

> Since reg vars wasn't really the focus of this patch, I mostly left the 
> existing text.  It may look new in the patch because it got moved into 
> this new menu.  I don't know why this was originally written this way 
> (sounds like laziness).  "Register" and "Variables" make more sense to 
> me.  That said, unless you feel strongly, I'd suggest doing this in my 
> upcoming RegVars patch.

Okay.

> As for the larger issue of "ISO C" vs "ISO C90", that's a tougher 
> question. Now that I've undone this change, all the inline asm stuff 
> says "ISO C," which is what the original text used.  "ISO C90" and "ISO 
> C99" are sprinkled throughout the other sections in extend.texi, and I'm 
> reluctant to make wholesale changes to the rest of the file as part of 
> this patch.

Let's leave the asm stuff at "ISO C" unless anyone has concerns
about this.

>>>> +By definition, a Basic @code{asm} statement is one with no operands.
>>>> +@code{asm} statements that contain one or more colons (used to
>>>> delineate
>>>> +operands) are considered to be Extended (for example, @code{asm("int
>>>> $3")}
>>>> +is Basic, and @code{asm("int $3" : )} is Extended). @xref{Extended
>>>> Asm}.
>> At this point the reader does not yet know the concept of those
>> colons, does she?  So that can be seen as a bit confusing.
> I understand what you mean, however I don't see a practical solution 
> here. Describing the use of colons in Basic asm, only to follow it by 
> saying "now that you understand all that, none of it applies here" seems 
> like it would be even more confusing.
> 
> Saying that colons are part of Extended asm, and providing a link to 
> Extended asm which does describe them seems to me the best compromise.

That'd work for me.

> What's more, while a sequential reading of the docs might experience the 
> problem you describe, does anyone actually read the docs from beginning 
> to end?  "At this point" is an undefined term.

I've seen both uses of our documentation.  This is not a biggie, I
am fine keeping it as is (or going with the compromise you suggest
above).

>>>> +This is a literal string that specifies the assembler code. The string
>>>> can
>> Just "assembler code", without "the", would be my take, but let's see
>> what native speakers so.
> "The" sounds right to me.  Alternate might be "thine?"  Maybe not.

Let's keep it as is, then.

>>>> +You may place multiple assembler instructions together in a single
>>>> @code{asm}
>>>> +string, separated by the characters normally used in assembly code for
>>>> the
>>>> +system.
>> Might "by the same characters normally...the target system" be more 
>> clear?
> Hmm.  Not to me.  I'm not sure what "same" would mean in this context.  
> The same as what?

Actually, reading this again after a couple of days, I'm not sure
what triggered my comment.  Now this works just fine.

>>>> +A combination that works in most places is a newline to break the
>>>> +line, plus a tab character to move to the instruction field (written
>>>> +as "\n\t").
>> Will everyone know what an instruction field is?  I'm not sure it's
>> that common of a term.
> Hmm.  I brought that text across unchanged from the original text. I 
> know what it means, but only because I've programmed in languages that 
> require it.  I haven't seen an assembler that cares, but my cross 
> platform experience is weak.
> 
> I don't want to get into the business of describing how to format and 
> write assembler code.  That's (well) beyond the scope of this doc.  
> What would you say to:
> 
> A combination that works in most places is a newline to break the
> line, plus a tab character (written as "\n\t").

Yep, that sounds good.

> If it seems to you I am trying to make Basic asm sound bad, I am. There 
> are so many things that "look right" when written in Basic asm that just 
> won't work, I'm trying my best to discourage its use.

Fair enough. :-)

>>>> +Since GCC does not parse the AssemblerInstructions, it has no
>>>> visibility of
>>                                 ^^^^^^^^^^^^^^^^^^^^^
>> Something wrong here.
> AssemblerInstructions is the first parameter to the Basic asm.

Right.  I think I was expecting this to be in @var{...}, but no
need to change this for now.  This could be part of a general
patch doing that consistently later on.

>> Should this be ``memory'' (note the quotes)?
> There's a reason I did it this way.  As an example, a common use for asm is:
> 
> asm ("" : : : "memory");
> 
> Since the memory clobber actually gets enclosed in quotes when used, I used
> that format when discussing it.  I suppose an argument could be made for any
> of:
> 
> memory
> "memory"
> @emph{memory}
> ``memory''
>
> but I like "memory".  If you have a strong feeling for one of these
> alternatives, I'll change it.  My recommendation is to leave it.

Or @code{memory} or code{"memory"}. ;-)  What do you think?  No need
to change if you prefer not to.

> Was there a question here?

No, just an extensive (excessive) quote.

>>>> +operands. To separate the classes of operands, you use colons. Basic
>>>> +@code{asm} statements contain no colons. (So, for example,
>>>> +@code{asm("int $3")} is Basic @code{asm}, and @code{asm("int $3" : )}
>>>> is
>>>> +Extended @code{asm}. @pxref{Basic Asm}.)
>> What does "classes of operands refer to"?  This does not seem to be
>> evident from the context.
> Looking at it in context:
> 
> asm [volatile] ( AssemblerTemplate : [OutputOperands] [ : [InputOperands] [ :
> [Clobbers] ] ] )
> 
> Looking at where the colons fall, they separate OutputOperands from
> InputOperands and InputOperands from Clobbers.  Observation suggests that
> makes "OutputOperands", "InputOperands" and "Clobbers" the classes in
> question.
> 
> On the other hand, I already knew what I meant.  If this is unclear, 
> what would you suggest?

I also guessed what this meant, still I am wondering whether there
might be a better term for this than classes.

>> We had a similary paragraph earlier on.  Perhaps only keep this where
>> it relates to Extended statements?  Not sure about this, though.
> The similar text was on the Basic Asm page.  It referenced the Extended 
> page. This text is on the Extended page, referencing back to Basic.  
> Since we can't know where in the docs the reader "started," each 
> section should acknowledge the other.

Absolutely, I am a big fan of cross-references.  I was wondering
whether we might be able to abstract things into a common part, but
that then makes things harder to read with a more tricky flow, so I
am fine leaving it as is.

>>>> +GCC's optimizer sometimes discards @code{asm} statements if it
>>>> determines
>>>> +that it has no need for the output variables.
>> How about just saying "GCC", not referring to the optimizer here?
> I'm pretty sure it only happens when running the optimizer.

There is just no such thing as "the GCC optimizer".  There are many 
optimizations that occur at different phases of translation, and 
individual passes, just not one clearly demarked and defined optimizer.
That was behind my comment.

>>> +@code{DoCheck} routine. By omitting the @code{volatile} qualifier when
>>> it
>>> +isn't needed you allow the optimizer to produce the most efficient code
>>> +possible.
>> How about "Only including ...when it is really necessary allows the
>> optimizer", that is, a more positive ("included" instead of "omitting")
>> approach?
> Hmm.  How strongly do you feel about this one?  I'm ok with the current 
> text.

The current one text is okay, I just think making this more "active"
and defaulting to _not_ using it as oppoed to assume usage and omitting
in case is a bit better.

>>>> +GCC's optimizer will not treat this code like the non-volatile code in
>>>> the
>>>> +earlier examples. It does not move it out of loops or omit it on the
>>>> +assumption that the result from a previous call is still valid.
>> "GCC will..."
> Again, I believe this behavior comes from the optimizer.

See above.

>>>> +@example
>>>> +@{ dialect0 | dialect1 | dialect2... @}
>>>> +@end example
>> :
> ?
> 
> Are you saying there is supposed to be a colon here somewhere?

No, that was just a contracted form of quoting I sometimes use as
opposed to the more correct
 > [...]
or similar.


>> @code{=}
>> 
>> @code{+}
>> 
>> Possibly under proper quotes as well?
> I tried it without quotes (@code{=}), and it looked really odd to my 
> eyes. The effect of the new font doesn't really show up for just a 
> single character like this.
> 
> I've changed this to @code{"="}.  Is that what you meant?

This is a question for Joseph.  I see how a single character
under @code{} won't work, yet @code{"="} doesn't feel right,
either.  Perhaps ``@code{=}''?

Let's defer this and raise with Joseph separately, unless he
responds here.

>> Local Reg Vars should be properly expanded, I assume.
> Well, the current title of this section really is Local Reg Vars. Unless 
> we want to expand this patch to start updating even more things...

I agree to get this patch in with minimal changes now and rather
follow up with other changes later on.

>>>> Accepts
>>>> +any (non-constant) variable within scope.
>> What's a non-constant variable?  Or just a constant variable?
> const int a = 4; /* A constant variable */
> int b = 5; /* A non-constant variable */
>
> In summary, anything that isn't marked as "const" is not const (aka
> non-constant).

Yeah, it's just a bit unintuitive to combine constant and variable
to describe one and the same entity.  If the language standards use
that, so be it.

>>>> + Either "=" or "+". If you must use a specific register, but your
>>>> Machine
>> Quotes.
> Not sure I'm understand here.  The quotes seem appropriate.  I could add 
> @code, but I'm not sure that's needed.

That's just another instance of the "Let's see what Joseph thinks"
item above.

>>>> +Input constraints can also be digits (for example, @code{"0"}). This
>>>> indicates
>> Quotes inside of @code do not seem approriate here.
> As I mentioned above, applying @code to a single character isn't 
> visually distinctive.  Though if you feel strongly, I'll change it.

Let's queue this with the other "Joseph" changes.

>>>> +constraint @code{"0"} for input operand 1 says that it must occupy the
>>>> same
>> Mind the quotes.
> As I mentioned above, applying @code to a single character isn't 
> visually distinctive.  Though if you feel strongly, I'll change it.

And this.

> To reference a label, prefix it with @code{%l} (that's a lowercase L) followed
> by its (zero-based) position in GotoLabels plus the number of input
> arguments.  For example, if the @code{asm} has three inputs and references two
> labels, refer to the first label as @code{%l3} and the second as @code{%l4}).

Sounds good.

>>>> +@multitable {Modifier} {Print the opcode suffix for the size of th}
>>>> {Operand} {masm=att} {masm=intel}
>> WHere does th come from here?
> Ahh.  I know a texi trick you don't?

That's not super hard. :-)

> These parameters to multitable aren't actually output into the text.  
> They are only used to provide a column width.  And if I put the entire 
> sentence here (from the table below), texi won't wrap the text in that 
> column.  Without wrapping, the text is too long for a single line when 
> output to pdf.  By truncating where I did, it uses a sensible column 
> width and wraps the text nicely.

Makes sense.  Thanks for the explanation!

>>>> +@headitem Modifier @tab Description @tab Operand @tab masm=att @tab
>>>> masm=intel
>> @code{masm...} ?
> You think?  It's not really code.  How about @option?

Yes!  That was me being stuck with too much web page work, where
we only have <code>.

> BTW, do you want the standard "diff" output?  Or diff -u?

Usually diff -u, though I can grok both.  Either way, in a number
of cases diff was not too helpful with this patch.  Not your fault
at all, and diff behaved as designed, as a human we would have made 
some different choices.


Where I did not comment in this reply of mine, I was good or there
was nothing I felt was open.

> I'd also like to make one last plug for having you at least page thru 
> one of the output formats (such as the html I linked to at the top).  
> While looking at the actual .texi lets you catch things like @ifhtml and 
> @cindex errors, I believe structure and formatting are clearer in html.  
> What's more, I feel confident saying more people will view the html text 
> than the texi, so a little extra effort to make sure the html is correct 
> is merited.

I'll try to do that at a later point in time.  Let's not delay the
patch over this.

Given their involvement from the technical side, I'd suggest that
Andrew or Richard commit the patch after the last final tweaks from
your side.

Thanks for your push to fix this up and your persistence!

Gerald


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]