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.

Sign up for GitHub

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

Jump to bottom

cmd/compile: delete the legacy compiler back end #16357

Closed
randall77 opened this issue Jul 13, 2016 · 32 comments
Closed

cmd/compile: delete the legacy compiler back end #16357

randall77 opened this issue Jul 13, 2016 · 32 comments
Labels
FrozenDueToAge
Milestone
Go1.8

Comments

@randall77
Copy link
Contributor

When all the SSA backends are done, delete the legacy compiler.

This is basically a tracking bug for the work on all the SSA backends.

amd64 - done
arm - #15365
arm64 - #16009
ppc64 - #16010 #16175
@randall77 randall77 added this to the Go1.8 milestone Jul 13, 2016
@randall77 randall77 self-assigned this Jul 13, 2016
@randall77
Copy link
Contributor Author

386 - #16358

@bradfitz
Copy link
Contributor

mips64, @cherrymui ?

@cherrymui
Copy link
Member

MIPS64- #16359

@randall77
Copy link
Contributor Author

amd64p32 is now using the SSA backend.

https://go-review.googlesource.com/c/25586/

@randall77
Copy link
Contributor Author

386 (including GO386=387) is now using the SSA backend.

@randall77
Copy link
Contributor Author

ARM is done.

@bradfitz bradfitz mentioned this issue Aug 12, 2016
Closed
@randall77
Copy link
Contributor Author

MIPS64 is done.

All that is left is s390x and ppc64(be).

@dr2chase
Copy link
Contributor

ppc64be lacks only a test machine.
Last time it was tested, it worked, or so I hear.

On Fri, Aug 26, 2016 at 4:51 PM, Keith Randall notifications@github.com
wrote:

MIPS64 is done.

All that is left is s390x and ppc64(be).

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#16357 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AB1vJ4p97fG7FUUW5Ww24JH4c5n0oeUkks5qj1HLgaJpZM4JLvsd
.

@billotosyr
Copy link

working on s390x

@randall77
Copy link
Contributor Author

arm64 is done.

@randall77
Copy link
Contributor Author

s390x is done #16677
We now have the greenlight to start deleting the legacy compiler.
Any suggestions on how to do the equivalent of GOSSAHASH when there is no longer a legacy compiler are welcome.

@bradfitz
Copy link
Contributor

@randall77, could GOSSAHASH remain but instead of using the old backend, mean to disable optimizations (like -N)?

@randall77
Copy link
Contributor Author

That would work if you're hacking on an SSA pass that is disabled with -N. That covers some cases but not all.
I want something like "clone the cmd/compile/internal/ssa package. Develop a CL on one of them. Use GOSSAHASH to decide per-function whether to use the orignal or the modified version".
Maybe include cmd/compile/internal/gc/ssa.go and cmd/compile/internal/ARCH/ssa.go in that clone/swap process.

@josharian
Copy link
Contributor

How about a top level flag (-hash=) and a support function (func hashmatch(name string) bool) that reports whether it matches. Then you can conditionally enable/disable any chunk of code you want, anywhere in the compiler.

@randall77
Copy link
Contributor Author

An individual phase would be pretty easy to hack in by hand, yes.
You just have to duplicate the code in question and add a flag check. It gets daunting when your change is across 3 packages.

@dr2chase
Copy link
Contributor

I've done Josh's method before, and I'm pretty happy with that method.
Some way of doing it wholesale old/new would be more than a little interesting.

@josharian
Copy link
Contributor

Back to the exciting topic of deleting the old backend, Keith, it seems to me you should have the honor of doing the first big del, if you want it.

@josharian
Copy link
Contributor

Another, much more complicated GOSSAHASH idea: Teach the linker to be able to combine two sources of object files. Using the known good compiler, compile everything and stash the object files. At link time, use GOSSAHASH to merge them. Wrap up behind a tool, maybe via -toolexec?

Sounds complicated, but maybe that's ok...or maybe it'll inspire a better idea in someone else.

@randall77
Copy link
Contributor Author

I am licking my lips.

On Tue, Sep 13, 2016 at 1:52 PM, Josh Bleecher Snyder <
notifications@github.com> wrote:

Back to the exciting topic of deleting the old backend, Keith, it seems to
me you should have the honor of doing the first big del, if you want it.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#16357 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AGkgIDR1ieJsp6xBnmCKp9h35xlfsMROks5qpw0dgaJpZM4JLvsd
.

@dr2chase
Copy link
Contributor

Given that we export all the phases as function pointers, this sort of old/new switchiness is perhaps not that magical. We might want to be more disciplined in our connections between gc/ssa.go and the ssa package.

@randall77
Copy link
Contributor Author

I like Josh's idea. Kind of like a per-function toolstash. A hacky Go
program that takes two pkg directories and merges each .o/.a using hashes
of the function names to decide which one to take.

On Tue, Sep 13, 2016 at 2:00 PM, dr2chase notifications@github.com wrote:

Given that we export all the phases as function pointers, this sort of
old/new switchiness is perhaps not that magical. We might want to be more
disciplined in our connections between gc/ssa.go and the ssa package.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#16357 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AGkgIKxnfZYjtqspnk5Dpsd6ved-lgXWks5qpw7QgaJpZM4JLvsd
.

@dr2chase
Copy link
Contributor

Ah, I misunderstood. Is that going to be easy/possible to embed within make.bash or go test?

@josharian
Copy link
Contributor

Ah, I misunderstood. Is that going to be easy/possible to embed within make.bash or go test?

You would presumably write a wrapper, like what buildall does for toolstash -cmp.

gopherbot pushed a commit that referenced this issue Sep 14, 2016
@randall77
cmd/compile: make ssa compilation unconditional
167e381 
Rip out the code that allows SSA to be used conditionally.

No longer exists:
 ssa=0 flag
 GOSSAHASH
 GOSSAPKG
 SSATEST

GOSSAFUNC now only controls the printing of the IR/html.

Still need to rip out all of the old backend.  It should no longer be
callable after this CL.

Update #16357

Change-Id: Ib30cc18fba6ca52232c41689ba610b0a94aa74f5
Reviewed-on: https://go-review.googlesource.com/29155
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
@gopherbot
Copy link

CL https://golang.org/cl/29168 mentions this issue.

gopherbot pushed a commit that referenced this issue Sep 15, 2016
@randall77
cmd/compile: delete lots of the legacy backend
e7cc9c4 
It's not everything, but it is a good start.

I tried to make the CL delete only.  goimports forced
a few exceptions to that rule.

Update #16357

Change-Id: I041925cb2fe68bb7ae1617af862b22c48da649c1
Reviewed-on: https://go-review.googlesource.com/29168
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Martin Möhrmann <martisch@uos.de>
@bradfitz
Copy link
Contributor

More deletes for the record:

24965bc
ad7732a
f03855f
82703f8

gopherbot pushed a commit that referenced this issue Sep 15, 2016
@randall77
test: make SSA tests unconditional
f13701b 
Delete legacy backend tests, make SSA tests unconditional.

Next CL will remove _ssa from the file names.

Update #16357

Change-Id: I2a7f5dcbc69455f63b5e6e6b2725df26ab86c8dd
Reviewed-on: https://go-review.googlesource.com/29231
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
gopherbot pushed a commit that referenced this issue Sep 15, 2016
@randall77
test,cmd/compile: remove _ssa file suffix
b265d51 
Everything is SSA now.

Update #16357

Change-Id: I436dbe367b863ee81a3695a7d653ba4bfc5b0f6c
Reviewed-on: https://go-review.googlesource.com/29232
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
@bradfitz
Copy link
Contributor

More:

f13701b
ae7e0ad
1f2930c
544010a
1bd91d4
9658de3

/cc @mdempsky

@mdempsky
Copy link
Member

Should I start including and suggesting others to add "Updates #16357" to cleanup CLs related to this?

@randall77
Copy link
Contributor Author

Yes, can't hurt.

On Fri, Sep 16, 2016 at 10:44 AM, Matthew Dempsky notifications@github.com
wrote:

Should I start including and suggesting others to add "Updates #16357
#16357" to cleanup CLs related to
this?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#16357 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AGkgIEe62Rip0btCljPzVpoPNfcNBnxBks5qqtVjgaJpZM4JLvsd
.

@mdempsky
Copy link
Member

Roger roger.

@gopherbot
Copy link

CL https://golang.org/cl/29350 mentions this issue.

gopherbot pushed a commit that referenced this issue Sep 16, 2016
@mdempsky
cmd/compile: remove unused Label fields
9f447c2 
Updates #16357.

Change-Id: I37f04d83134b5e1e7f6ba44eb9a566758ef594d3
Reviewed-on: https://go-review.googlesource.com/29350
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
gopherbot pushed a commit that referenced this issue Sep 16, 2016
@mdempsky
cmd/compile: remove Arch.REGCTXT
08feadb 
Update #16357.

Change-Id: I507676212d7137a62c76de7bfa0ba8dbd68e840f
Reviewed-on: https://go-review.googlesource.com/29358
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Dave Cheney <dave@cheney.net>
gopherbot pushed a commit that referenced this issue Sep 17, 2016
@mdempsky
cmd/internal/obj: remove ACHECKNIL
246074d 
Updates #16357.

Change-Id: I35f938d675ca5c31f65c4419ee0732bbc593b5cb
Reviewed-on: https://go-review.googlesource.com/29368
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Dave Cheney <dave@cheney.net>
TryBot-Result: Gobot Gobot <gobot@golang.org>
@gopherbot
Copy link

CL https://golang.org/cl/29219 mentions this issue.

gopherbot pushed a commit that referenced this issue Sep 21, 2016
@mdempsky
cmd/compile: simplify obj.ProgInfo and extract from obj.Prog
e613470 
Updates #16357.

Change-Id: Ia837dd44bad76931baa9469e64371bc253d6694b
Reviewed-on: https://go-review.googlesource.com/29219
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
gopherbot pushed a commit that referenced this issue Sep 29, 2016
@ALTree @bradfitz
cmd/compile: delete unused (*Node) SetInt, SetBigInt, Bool
2b49d12 
Introduced in CL 9263 (prepare to unexport gc.Mp*) and CL 9267
(prepare Node.Val to be unexported), their only callers were in
the old backend and all got deleted in CL 29168 (cmd/compile:
delete lots of the legacy backend).

Update #16357

Change-Id: I0a5d76b98b418e8ec0984c033c3bc0ac3fc5f38a
Reviewed-on: https://go-review.googlesource.com/29997
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
gopherbot pushed a commit that referenced this issue Sep 30, 2016
@ALTree @bradfitz
cmd/compile: delete unused Convconst function
41fa42b 
Convconst is not used in the new backend, and all its callers
were deleted in CL 29168 (cmd/compile: delete lots of the legacy
backend). iconv was an helper function for Convconst.

Updates #16357

Change-Id: I65c7345586d7af81cdc2fb09c68f744ffb161a17
Reviewed-on: https://go-review.googlesource.com/30090
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
@randall77
Copy link
Contributor Author

Declaring this done.

@rsc rsc changed the title cmd/compile: delete the legacy compiler cmd/compile: delete the legacy compiler back end Oct 17, 2016
@golang golang locked and limited conversation to collaborators Oct 17, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
Go1.8
Development

No branches or pull requests
8 participants
@bradfitz @mdempsky @josharian @dr2chase @randall77 @gopherbot @billotosyr @cherrymui