code review 6495074: cmd/gc: add commentary to the OXXX constants.

cmd/gc: add commentary to the OXXX constants.

[nigeltao, 2012-09-03 07:34:20]

Hello rsc@golang.org (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://code.google.com/p/go

[DMorsing, 2012-09-03 09:21:08]

This is great. I really could have used this when I first started hacking on gc.

http://codereview.appspot.com/6495074/diff/4002/src/cmd/gc/go.h
File src/cmd/gc/go.h (right):

http://codereview.appspot.com/6495074/diff/4002/src/cmd/gc/go.h#newcode561
src/cmd/gc/go.h:561: OINDREG,	// stack spill of an arch-specific register??
Indirection through register. zero value is stack pointer

[nigeltao, 2012-09-04 04:42:15]

https://codereview.appspot.com/6495074/diff/4002/src/cmd/gc/go.h
File src/cmd/gc/go.h (right):

https://codereview.appspot.com/6495074/diff/4002/src/cmd/gc/go.h#newcode561
src/cmd/gc/go.h:561: OINDREG,	// stack spill of an arch-specific register??
On 2012/09/03 09:21:08, DMorsing wrote:
> Indirection through register. zero value is stack pointer

Ah, of course, this is something like 8(SP). However, I don't think that the
zero value of node->val.u.reg is the stack pointer: D_SP is 20, D_AL is 0.

[DMorsing, 2012-09-04 06:23:12]

On 2012/09/04 04:42:15, nigeltao wrote:
> zero value of node->val.u.reg is the stack pointer: D_SP is 20, D_AL is 0.

Ah, I appear to be mistaken. Sorry about that.

[DMorsing, 2012-09-04 08:32:50]

I just thought of something. It might be helpful to use the naming of the
elements to convey how the nodes occur in the AST.

For example
// OADD l + r
// OAS2 l1, l2, l3 = r1, r2, r3
// OLEN len(l)

On the downside, It would throw away a bit of context in cases like OCMPSTR and
It's quite a bit more work.

[rsc, 2012-09-04 20:08:23]

LGTM

[nigeltao, 2012-09-04 23:34:14]

On 4 September 2012 18:32,  <daniel.morsing@gmail.com> wrote:
> I just thought of something. It might be helpful to use the naming of
> the elements to convey how the nodes occur in the AST.
>
> For example
> // OADD l + r
> // OAS2 l1, l2, l3 = r1, r2, r3
> // OLEN len(l)
>
> On the downside, It would throw away a bit of context in cases like
> OCMPSTR and It's quite a bit more work.

That might be useful, but they might be better as higher level
comments for groups of opcodes. For example, all binary ops are "l op
r", all unary ops are "op l", etc. I don't think that the order of the
values matter, apart from OXXX being first and OEND being last.
Something to consider for a follow-up CL, perhaps.

[nigeltao, 2012-09-04 23:35:10]

*** Submitted as http://code.google.com/p/go/source/detail?r=957c047e5586 ***

cmd/gc: add commentary to the OXXX constants.

R=rsc, daniel.morsing
CC=golang-dev
http://codereview.appspot.com/6495074