Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd/compile: add a README section on developing the compiler #30074

Open
mvdan opened this issue Feb 4, 2019 · 18 comments
Open

cmd/compile: add a README section on developing the compiler #30074

mvdan opened this issue Feb 4, 2019 · 18 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. Documentation NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@mvdan
Copy link
Member

mvdan commented Feb 4, 2019

For example, such a section could mention:

Anything else that comes to mind, please add it to this issue. I plan on sending a CL to add a markdown section sometime during the 1.13 cycle.

I realise the SSA package has lots of extra checks and debugging flags one can enable, but I think those should be covered by cmd/compile/internal/ssa/README. I am also not nearly as familiar with those as I'm with this list here, so I'm leaving ssa's tips for another issue/CL.

/cc @josharian @mdempsky @aclements @randall77

@mvdan mvdan self-assigned this Feb 4, 2019
@mvdan mvdan added this to the Go1.13 milestone Feb 4, 2019
@josharian
Copy link
Contributor

Wrapper tools: toolstash-check and compilecmp. (Maybe those should move to x/tools?)

@josharian
Copy link
Contributor

Maybe references to gosmith and how to fuzz the compiler and how to build and run a race-enabled compiler. Mention the SSA rulelog? Some basic tips (like start with the simplest possible reproduction and understand exactly what is happening with it). Pointers on where all the tests live (some in-package, some over in the test dir).

@mvdan
Copy link
Member Author

mvdan commented Feb 5, 2019

toolstash-check

Hmm, I've personally found it a bit unnecessary. What I do is always run GOROOT=/usr/lib/go ./make.bash && toolstash save, so then I always have a stashed set of tools which correspond to the current master branch I'm on.

Not exactly the same, as toolstash-check compares with the direct parent, but close enough :)

and compilecmp

I think it would need documentation if we are to suggest it here :)

Maybe references to gosmith and how to fuzz the compiler and how to build and run a race-enabled compiler.

I worry that these would be a bit too advanced for the "modifying the compiler" intro section. We don't want to bombard new developers with two pages of tools to learn. Perhaps this could fall under a more advanced section.

Mention the SSA rulelog?

Sure, but wouldn't this go in the SSA readme?

Some basic tips (like start with the simplest possible reproduction and understand exactly what is happening with it). Pointers on where all the tests live (some in-package, some over in the test dir).

Good ideas! I was forgetting about the simpler pieces of advice like these.

@mvdan
Copy link
Member Author

mvdan commented Feb 5, 2019

Here's another idea: How to measure whether a compiler patch produces smaller binaries. For example, if the prove pass is made a bit smarter, and we want to check that it's really removing many bounds checks from a large binary.

I believe this is usually measured by comparing text section sizes between two built binaries, but I'm not sure what the tips for this are. For example:

  • What large binaries are generally good for this? cmd/go, cmd/gofmt, cmd/compile?
  • Which (portable) tool should be used to extract the sizes? size, readelf, or go tool something?
  • Is this process automatable in any way?

@josharian
Copy link
Contributor

toolstash-check
Hmm, I've personally found it a bit unnecessary.

It helps prevent making silly mistakes, which I have personally done more than I care to recount. Also, it makes it easy to check all platforms, which matters sometimes.

I think it would need documentation if we are to suggest it here :)

Yes. Mea culpa. Although FWIW most folks I have recommended it to (on CLs/issues) figure it out pretty readily. It could also use a bit of cleanup.

I have also long had plans to unify some of the toolchain-wranging in compilecmp and toolstash-check and pull it into a re-usable package. I have some of it written, but never finished it.

How to measure whether a compiler patch produces smaller binaries.

compilecmp does a fair amount of this already. If you pass it -obj, which could be made to default to on, it inspects the object files produced by the compiler and tells you about changes, separating out code and static symbols from the export data. It also does a check on the size of hello world, although that's not really very informative. But the infrastructure to do this is all in compilecmp.

@mdempsky
Copy link
Contributor

mdempsky commented Feb 5, 2019

Hmm, I've personally found it a bit unnecessary. What I do is always run GOROOT=/usr/lib/go ./make.bash && toolstash save, so then I always have a stashed set of tools which correspond to the current master branch I'm on.

For rapid development, I usually just toolstash directly as well, but when I'm working on multi-stage CLs where some (but not all) of the CLs affect compiler output, it's easy for me to lose track of what toolchain I stashed. So toolstash-check is my error-proof pre-upload check.

Since it builds in /tmp, you can also check multiple CLs in parallel. And like @josharian mentioned, it can help checking other platforms too.

  • General recommendations like toolstash restore && go install cmd/compile to rebuild the compiler in a fast and stable way

FWIW, I usually use go install -toolexec=toolstash cmd/compile.

--

Some other things to mention: how to inspect various stages of the compiler. For example, using -S to dump assembly output; GOSSAFUNC to dump SSA stuff; -W to dump the typechecked trees; etc.

@josharian
Copy link
Contributor

@mvdan want to write some docs, call this completed, or bump to 1.14?

@mvdan
Copy link
Member Author

mvdan commented May 3, 2019

I do intend to work on this, ideally for 1.13. I assume it's fine to review and submit during the first half of the freeze, as it's only documentation that doesn't affect the release. But if people disagree, it can wait until 1.14.

@randall77
Copy link
Contributor

Documentation during the freeze is actively encouraged.

@mvdan
Copy link
Member Author

mvdan commented May 3, 2019

Great! I'm currently at GopherCon Singapore, so I'll probably get to this later in May.

@josharian
Copy link
Contributor

A section about the -d flags would be helpful. See conversation at CL 176718.

@andybons andybons modified the milestones: Go1.13, Go1.14 Jul 8, 2019
@rsc rsc modified the milestones: Go1.14, Backlog Oct 9, 2019
@dgryski
Copy link
Contributor

dgryski commented Aug 29, 2020

@dmitshur dmitshur added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Sep 11, 2020
@mvdan
Copy link
Member Author

mvdan commented Mar 23, 2022

I'll probably get to this later in May.

I evidently did not get to this, and I haven't done any compiler development in a while, so this is up for grabs. Ideally by someone who actually keeps up to date with how to do compiler development nowadays.

@mvdan mvdan removed their assignment Mar 23, 2022
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/404694 mentions this issue: cmd/compile: update README.md

gopherbot pushed a commit that referenced this issue May 10, 2022
This CL updates the description of the frontend packages of the
compiler, which I'm more familiar with.

Updates #30074.

Change-Id: Ic279007f6102b21072d6558fa9035f0fcc267d93
Reviewed-on: https://go-review.googlesource.com/c/go/+/404694
Reviewed-by: Keith Randall <[email protected]>
Reviewed-by: David Chase <[email protected]>
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Jul 13, 2022
@mknyszek mknyszek moved this to Triage Backlog in Go Compiler / Runtime Jul 15, 2022
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/503895 mentions this issue: cmd/compile: add a 'Tips' section to README to help new contributors

@thepudds
Copy link
Contributor

thepudds commented Jul 7, 2023

FYI, I sent CL 503895, which adds a new "Tips" section to the cmd/compile README.

I initially built it up as a classic "things I wish I knew a month ago", and hence is mostly focused on new-ish cmd/compile contributors.

I fleshed it out by cross-checking against topics mentioned above. The CL currently at least touches on many of them, but not:

Wrapper tools: toolstash-check and compilecmp. (Maybe those should move to x/tools?)

Maybe references to gosmith and how to fuzz the compiler and how to build and run a race-enabled compiler.

How to measure whether a compiler patch produces smaller binaries

See conversation at CL 176718.

I can add more to the CL, though perhaps more advanced topics can be left to the future.

gopherbot pushed a commit that referenced this issue Sep 12, 2023
This CL adds a new 'Tips' section to the cmd/compile README.

The primary intent is to help new-ish contributors.

It includes some basics on getting started, testing changes,
viewing coverage, juggling different compiler versions,
some links to additional tools, and so on.

Updates #30074

Change-Id: I393bf1137db9d2bb851f7e254b08455273ccad8c
Reviewed-on: https://go-review.googlesource.com/c/go/+/503895
TryBot-Result: Gopher Robot <[email protected]>
Reviewed-by: Matthew Dempsky <[email protected]>
Run-TryBot: t hepudds <[email protected]>
Reviewed-by: Keith Randall <[email protected]>
Reviewed-by: Keith Randall <[email protected]>
@thepudds
Copy link
Contributor

thepudds commented Apr 10, 2024

Hi @alandonovan, a brief follow-up on our discussion just now — it would be helpful I think if there was a short paragraph on export data (ideally with a few outbound links) added to cmd/compile/README.md.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/578055 mentions this issue: cmd/compile: describe export data

gopherbot pushed a commit that referenced this issue May 7, 2024
Updates #30074

Change-Id: Ic74d482943d992c20f69edb106c666a7b26291c1
Reviewed-on: https://go-review.googlesource.com/c/go/+/578055
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Than McIntosh <[email protected]>
Reviewed-by: Cuong Manh Le <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. Documentation NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
Status: Triage Backlog
Development

No branches or pull requests

10 participants