Re: code review 20930043: doc/asm.html: new document, a brief guide to the assembler (issue 20930043)

[minu...@gmail.com]

https://codereview.appspot.com/20930043/diff/120001/doc/asm.html
File doc/asm.html (right):

https://codereview.appspot.com/20930043/diff/120001/doc/asm.html#newcode152
doc/asm.html:152: and the frame size as a literal constant (that is, a
value prefixed by <code>$</code>).
actually not one constants, but one constant and optionally another one.
(framesize - argsize). And it is very important to give the correct
argsize
here or the backtrace routine will complain.

we might also want to document what $-4 does for the frame size on ARM
(omit the LR save).

https://codereview.appspot.com/20930043/diff/120001/doc/asm.html#newcode316
doc/asm.html:316: The value of <code>m</code> is stored in
<code>0(GS)</code>, <code>g</code> in <code>8(GS)</code>.
On 2013/11/02 12:15:57, aram wrote:
> > this is not valid on most systems anymore.... esp. plan 9 and
> > windows. > again, this is very low level detail and it's subject
> > to change from release to release, so I think we should avoid
> > introducing them.
> On the contrary, without knowing this is impossible to do any kind
> of runtime work. I think we should document the mechanism and keep
> the description up to date when things change.
I think the document is meant for 3rd-party package authors who might
want to use assembly to address a hot-spot in his package.

For those involved with the runtime, they probably need to figure a lot
out by themselves.

I think we certainly don't want 3rd-party packages to mess with m and g
(even just reading fields from them), because:
1. they can't include zasm_GOOS_GOARCH.h, which provides symbolic
constants for offsets of the fields.
2. they are not stable between releases, and the compiler won't tell
them
when we break their code. And because of 1, the problem is more severe.

> At the very least we should point to
/src/cmd/dist/buildruntime.c:/zasmhdr
even if we mention zasm header file, out-of-tree packages can't include
them.

I suggest we maintain a page on go-wiki to say something about the
runtime
and toolchain, but don't go too deep in this general assembly article,
also we
probably should put a disclaimer saying that using any fields from m and
g
is dangerous and might break without notice.

https://codereview.appspot.com/20930043/

[r...@golang.org]

https://codereview.appspot.com/20930043/diff/120001/doc/asm.html
File doc/asm.html (right):

https://codereview.appspot.com/20930043/diff/120001/doc/asm.html#newcode39
doc/asm.html:39: <a href="/pkg/runtime/"><code>runtime</code></a> and

On 2013/11/02 12:15:57, aram wrote:

> Agreed.

i've linked to /pkg/runtime in all other docs.

https://codereview.appspot.com/20930043/diff/120001/doc/asm.html#newcode105
doc/asm.html:105: Some symbols, such as <code>PC</code>, <code>R0</code>
and <code>SP</code>, are predeclared and refer to registers.
i do not fell confident to document this. if you want to give me text,
we can put it in the ARM section.

https://codereview.appspot.com/20930043/diff/120001/doc/asm.html#newcode126
doc/asm.html:126: that assembly programming is a fraught endeavor.
On 2013/11/02 06:14:19, minux wrote:
> except the m, g registers in ARM though. the convention is lower case.
> (and the assembler will only recognize the lower case one)

> another inconsistency, perhaps we should fix it before release however
> the C runtime code also uses lower cased m and g, so I'm not sure if
> we want to break the consistency here.

Done.

https://codereview.appspot.com/20930043/diff/120001/doc/asm.html#newcode140
doc/asm.html:140: so to convert that output to assembler input they must
be translated back into middle dots.
On 2013/11/02 06:14:19, minux wrote:
> do you also want to mention / (U+2215)?

> it allows you to refer to symbols in other packages that have slashes
in their
> package names.

> for example, see src/pkg/runtime/race.c

maybe in the next round.

https://codereview.appspot.com/20930043/diff/120001/doc/asm.html#newcode150
doc/asm.html:150: The last instruction in a <code>TEXT</code> block must
be some sort of jump, usually a <code>RET</code> (pseudo-)instruction.
On 2013/11/02 06:14:19, minux wrote:
> if it's not, the linker will add a jump to itself catchall instruction
for you,
> so don't expect to achieve fall through TEXT functions.

Done.

https://codereview.appspot.com/20930043/diff/120001/doc/asm.html#newcode152
doc/asm.html:152: and the frame size as a literal constant (that is, a
value prefixed by <code>$</code>).

On 2013/11/03 08:45:48, minux wrote:
> actually not one constants, but one constant and optionally another
one.
> (framesize - argsize). And it is very important to give the correct
argsize
> here or the backtrace routine will complain.

> we might also want to document what $-4 does for the frame size on ARM
> (omit the LR save).

done

https://codereview.appspot.com/20930043/diff/120001/doc/asm.html#newcode207
doc/asm.html:207: Their values are:
maybe for after go1.2

https://codereview.appspot.com/20930043/diff/120001/doc/asm.html#newcode207
doc/asm.html:207: Their values are:
On 2013/11/02 06:14:19, minux wrote:
> the assembly file must include textflag.h in cmd/ld to use those
symbolic names,
> which makes it very diffcuilt for 3rd-party (out-of-tree) packages to
use them.

> Perhaps we should do something here.

> i suggest we make it one of the standard header and make cmd/dist copy
it
> to pkg/$GOOS_$GOARCH just like runtime.h and cgocall.h.

> (I will make CL for this, if you agree)

Done.

https://codereview.appspot.com/20930043/diff/120001/doc/asm.html#newcode259
doc/asm.html:259: That is, the file
<code>$GOROOT/src/cmd/8l/8.out.h</code> contains a C enumeration, called
<code>as</code>,
On 2013/11/02 06:14:19, minux wrote:
> <a href="/src/cmd/8l/8.out.h"><code>$GOROOT/...</code></a>

why? i state this is for 8.

this is an assembler guide (not manual even). it requires some knowledge
on the part of the reader. i'm not even planning to link it other than
from the assembler doc.go files.

https://codereview.appspot.com/20930043/diff/120001/doc/asm.html#newcode299
doc/asm.html:299: through the value of an otherwise unused register in
the MMU.

On 2013/11/02 12:15:57, aram wrote:

> I know you mean "unused for something else", but it can be interpreted
> as "unused in general". This paragraph, while correct, is misleading
> to someone accustomed to non-Go assembly programming. FS/GS usually
> point to the user-space thread structure, and from there programs
> can use TLS. On most platforms we reset FS/GS to something else
> with a private system call because the default doesn't work correctly
> with statically compiled binaries (the mechanism requires a lot of
> dance between the kernel, libc and ld.so). Even with our resetting,
> we still use them for the same purpose.

> We don't reset FS.base on Windows or on the nascent solaris port
> because these ports always produce dynamically linked binaries.

i think it's fine as is. this isn't a full assembler manual but i put in
a parenthetical

https://codereview.appspot.com/20930043/diff/120001/doc/asm.html#newcode300
doc/asm.html:300: A OS-dependent macro <code>get_tls</code> is
predefined in the assembler; it loads its argument register
On 2013/11/02 06:14:19, minux wrote:
> the file needs to include zasm_GOOS_GOARCH.h to use these macros.
> so 3rd-party (out-of-tree) packages are not able to use them.

done

> (In general, I think we should discourage 3rd-party package to mess
> with m and g, and I suggest we don't mention this)

it needs to be written down.

https://codereview.appspot.com/20930043/diff/120001/doc/asm.html#newcode308
doc/asm.html:308: MOVL m(CX), BX // Move m into BX.

On 2013/11/02 12:15:57, aram wrote:

> Could you reverse this line so that it's obvious that you can store
with these
> macros as well?

you shouldn't be writing the m and g registers and f you need to have it
explained how to implement m->x and this example isn't enough, you
shouldn't be here at all.

https://codereview.appspot.com/20930043/diff/120001/doc/asm.html#newcode315
doc/asm.html:315: in a two-word structure pointed to by the
<code>GS</code> machine register.

On 2013/11/02 12:15:57, aram wrote:

> GS is a pseudo-register now.

i don't know what that means in this context. i got this information
from reading the source code. if someone wants to provide accurate
information for this section, you're welcome to do so.

https://codereview.appspot.com/20930043/diff/120001/doc/asm.html#newcode332
doc/asm.html:332: </p>

On 2013/11/02 12:15:57, aram wrote:

> Agreed.

Done.

https://codereview.appspot.com/20930043/diff/120001/doc/asm.html#newcode332
doc/asm.html:332: </p>
On 2013/11/02 06:14:19, minux wrote:
> Also worth mentioning how to workaround unsupported opcodes with
> BYTE $0x0f; BYTE $0x6f; BYTE $0x00 // on x86 and
> WORD $0x12345678 // on ARM.

> This is very useful and in fact, i believe that most of 3rd-party
assembly
> code needs this (to use newest processor features for example).

Done.

https://codereview.appspot.com/20930043/

[r...@golang.org]

Hello golan...@googlegroups.com, ia...@golang.org, brad...@golang.org,
josh...@gmail.com, minu...@gmail.com, ar...@mgk.ro (cc:
golan...@googlegroups.com),

Please take another look.


https://codereview.appspot.com/20930043/

[minu...@gmail.com]

https://codereview.appspot.com/20930043/diff/120001/doc/asm.html
File doc/asm.html (right):

https://codereview.appspot.com/20930043/diff/120001/doc/asm.html#newcode105
doc/asm.html:105: Some symbols, such as <code>PC</code>, <code>R0</code>
and <code>SP</code>, are predeclared and refer to registers.

On 2013/11/04 22:33:56, r wrote:
> i do not fell confident to document this. if you want to give me text,
we can
> put it in the ARM section.

Let's do that in follow-up round.

https://codereview.appspot.com/20930043/diff/160001/doc/asm.html
File doc/asm.html (right):

https://codereview.appspot.com/20930043/diff/160001/doc/asm.html#newcode154
doc/asm.html:154: and the frame size as a constant (that is, an integer
expression prefixed by <code>$</code>).
TEXT name,flag,$framesize-argsize

https://codereview.appspot.com/20930043/diff/160001/doc/asm.html#newcode361
doc/asm.html:361: Here's how the 386 runtime defines the 64-bit atomic
load function.
similarly use WORD $0x01234567 for ARM.

https://codereview.appspot.com/20930043/

[minux]

In the next round, I suggest we add these topics:

1. consider document Go ABI (everything on stack, uintptr alignment before return values)

2. document //go:noescape

For the ARM specific section:

0. ARM instruction mnemonic format (MOVW.NE, etc)

1. multiple register move instructions suffixes (.W, .P)

2. pseudo instructions synthesized by the linker (esp. MOD/DIV), the use of R11

3. FP, SP pseudo register

4. always verify your assembly with a disassembler (esp. for ARM).

Any other topics that I'm missing here?

I think point 0 and 1 are documented in the plan 9 assembly docs, so perhaps

we only need to do the remaining points in this document.

[r...@golang.org]

LGTM

https://codereview.appspot.com/20930043/diff/160001/doc/asm.html
File doc/asm.html (right):

https://codereview.appspot.com/20930043/diff/160001/doc/asm.html#newcode51
doc/asm.html:51: $ go tool 6g -S x.go
# or: go build -gcflags -S x.go

https://codereview.appspot.com/20930043/diff/160001/doc/asm.html#newcode78
doc/asm.html:78: $ go tool 6l -a x.6
# or: go build -ldflags -a x.go

https://codereview.appspot.com/20930043/diff/160001/doc/asm.html#newcode154
doc/asm.html:154: and the frame size as a constant (that is, an integer
expression prefixed by <code>$</code>).

On 2013/11/07 03:59:05, minux wrote:
> TEXT name,flag,$framesize-argsize

What minux means is that the general form of the frame size is
$framesize-argsize, which says that the stack frame is framesize and the
function expects argsize bytes of arguments. Functions that might split
their stack (those without NOSPLIT) must specify argsize for the split
to work correctly.

https://codereview.appspot.com/20930043/diff/160001/doc/asm.html#newcode201
doc/asm.html:201: declares <code>runtime·tlsoffset</code> to have size
4.
The GLOBL instruction must follow the corresponding DATA instructions.

https://codereview.appspot.com/20930043/diff/160001/doc/asm.html#newcode280
doc/asm.html:280: represents the <code>ADCB</code> (add carry immediate
byte) instruction.
s/immediate //

https://codereview.appspot.com/20930043/diff/160001/doc/asm.html#newcode325
doc/asm.html:325: The runtime pointers to the <code>m</code> and
<code>g</code> structures are held
This should be considered the same as 386.
It is true that the get_tls argument is ignored on all current amd64
systems, but it is also ignored on some 386 systems. That detail depends
on the operating system, and it is plausible that some OS will come
along on amd64 that will require the register to be used, not ignored.

https://codereview.appspot.com/20930043/

[r...@golang.org]

https://codereview.appspot.com/20930043/diff/160001/doc/asm.html
File doc/asm.html (right):

https://codereview.appspot.com/20930043/diff/160001/doc/asm.html#newcode51
doc/asm.html:51: $ go tool 6g -S x.go

On 2013/11/07 16:27:56, rsc wrote:
> # or: go build -gcflags -S x.go

Done.

https://codereview.appspot.com/20930043/diff/160001/doc/asm.html#newcode78
doc/asm.html:78: $ go tool 6l -a x.6

On 2013/11/07 16:27:56, rsc wrote:
> # or: go build -ldflags -a x.go

Done.

https://codereview.appspot.com/20930043/diff/160001/doc/asm.html#newcode201
doc/asm.html:201: declares <code>runtime·tlsoffset</code> to have size
4.

On 2013/11/07 16:27:56, rsc wrote:
> The GLOBL instruction must follow the corresponding DATA instructions.

Done.

https://codereview.appspot.com/20930043/diff/160001/doc/asm.html#newcode280
doc/asm.html:280: represents the <code>ADCB</code> (add carry immediate
byte) instruction.

On 2013/11/07 16:27:56, rsc wrote:
> s/immediate //

Done.

https://codereview.appspot.com/20930043/diff/160001/doc/asm.html#newcode325
doc/asm.html:325: The runtime pointers to the <code>m</code> and
<code>g</code> structures are held

On 2013/11/07 16:27:56, rsc wrote:
> This should be considered the same as 386.
> It is true that the get_tls argument is ignored on all current amd64
systems,
> but it is also ignored on some 386 systems. That detail depends on the
operating
> system, and it is plausible that some OS will come along on amd64 that
will
> require the register to be used, not ignored.

Done.

https://codereview.appspot.com/20930043/diff/160001/doc/asm.html#newcode361
doc/asm.html:361: Here's how the 386 runtime defines the 64-bit atomic
load function.

On 2013/11/07 03:59:05, minux wrote:

> similarly use WORD $0x01234567 for ARM.

Done.

https://codereview.appspot.com/20930043/

[r...@golang.org]

Hello golan...@googlegroups.com, ia...@golang.org, brad...@golang.org,
josh...@gmail.com, minu...@gmail.com, ar...@mgk.ro, r...@golang.org