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: internal compiler error: cannot inline function containing closure #19705

Closed
ALTree opened this issue Mar 24, 2017 · 4 comments

Comments

Projects
None yet
3 participants
@ALTree
Copy link
Member

commented Mar 24, 2017

go version devel +5e954047bc Fri Mar 24 20:07:15 2017 +0000 linux/amd64

The following program:

package p

func f1() {
	f2()
}

func f2() {
	if false {
		_ = func() {}
	}
}

crashes the compiler on tip, but not in go1.8 and older.

./0.go:4:4: internal compiler error: cannot inline function containing closure: 
.   CLOSURE l(9) ff(1) tc(1) p.f2.func1 FUNC-func()

goroutine 1 [running]:
runtime/debug.Stack(0x0, 0x0, 0x0)
	/home/alberto/go/src/runtime/debug/stack.go:24 +0x79
cmd/compile/internal/gc.Fatalf(0xad5716, 0x2e, 0xc4203048a0, 0x1, 0x1)
	/home/alberto/go/src/cmd/compile/internal/gc/subr.go:175 +0x230
cmd/compile/internal/gc.(*inlsubst).node(0xc420305380, 0xc4200b23c0, 0xc42000f500)
	/home/alberto/go/src/cmd/compile/internal/gc/inl.go:1031 +0xd2f
cmd/compile/internal/gc.(*inlsubst).node(0xc420305380, 0xc4202dfe60, 0x1)
	/home/alberto/go/src/cmd/compile/internal/gc/inl.go:1035 +0x626
cmd/compile/internal/gc.(*inlsubst).list(0xc420305380, 0xc4202d5d60, 0x0, 0x0, 0xe76a08)
	/home/alberto/go/src/cmd/compile/internal/gc/inl.go:952 +0xd1
cmd/compile/internal/gc.(*inlsubst).node(0xc420305380, 0xc4202dfc20, 0x1)
	/home/alberto/go/src/cmd/compile/internal/gc/inl.go:1039 +0x841
cmd/compile/internal/gc.(*inlsubst).list(0xc420305380, 0xc420306040, 0xc4202f6800, 0xc42030a360, 0xc4202da6c0)
	/home/alberto/go/src/cmd/compile/internal/gc/inl.go:952 +0xd1
cmd/compile/internal/gc.mkinlcall1(0xc42030a240, 0xc4202f6400, 0x0, 0xa37bc0)
	/home/alberto/go/src/cmd/compile/internal/gc/inl.go:829 +0xb6d
cmd/compile/internal/gc.mkinlcall(0xc42030a240, 0xc4202f6400, 0xc420305600, 0xe77e80)
	/home/alberto/go/src/cmd/compile/internal/gc/inl.go:547 +0x78
cmd/compile/internal/gc.inlnode(0xc42030a240, 0x1)
	/home/alberto/go/src/cmd/compile/internal/gc/inl.go:509 +0x460
cmd/compile/internal/gc.inlnodelist(0xc420306120)
	/home/alberto/go/src/cmd/compile/internal/gc/inl.go:380 +0x55
cmd/compile/internal/gc.inlnode(0xc4200b2140, 0x2)
	/home/alberto/go/src/cmd/compile/internal/gc/inl.go:486 +0x2e1
cmd/compile/internal/gc.inlcalls(0xc4200b2140)
	/home/alberto/go/src/cmd/compile/internal/gc/inl.go:336 +0x50
cmd/compile/internal/gc.Main.func1(0xc420306020, 0x1, 0x4, 0xc4202fc800)
	/home/alberto/go/src/cmd/compile/internal/gc/main.go:465 +0x7f
cmd/compile/internal/gc.(*bottomUpVisitor).visit(0xc420305ab0, 0xc4200b2140, 0x0)
	/home/alberto/go/src/cmd/compile/internal/gc/esc.go:106 +0x259
cmd/compile/internal/gc.visitBottomUp(0xc4202d5fe0, 0x3, 0x4, 0xadb438)
	/home/alberto/go/src/cmd/compile/internal/gc/esc.go:62 +0xe7
cmd/compile/internal/gc.Main(0xadb350)
	/home/alberto/go/src/cmd/compile/internal/gc/main.go:456 +0x26cd
main.main()
	/home/alberto/go/src/cmd/compile/main.go:49 +0x95

This may be related to #19679.

@ALTree ALTree added this to the Go1.9 milestone Mar 24, 2017

@josharian

This comment has been minimized.

Copy link
Contributor

commented Mar 24, 2017

This is pretty easy to fix directly, but probably should be fixed by fixing #19699 instead.

@ALTree

This comment has been minimized.

Copy link
Member Author

commented Mar 25, 2017

Ok, leaving to you to decide if you want to merge this with #19699.

@josharian

This comment has been minimized.

Copy link
Contributor

commented Mar 25, 2017

Unlike #19699, this has a concrete problem and a reproducer, and it admits of a narrower fix, so I want to leave it open.

@gopherbot

This comment has been minimized.

Copy link

commented Mar 28, 2017

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

josharian added a commit to josharian/go that referenced this issue Apr 11, 2017

cmd/compile: eliminate dead code in if statements after typechecking
This is a more thorough and cleaner fix
than doing dead code elimination separately
during inlining, escape analysis, and export.

Unfortunately, it does add another full walk of the AST.
The performance impact is very small, but not non-zero.

If a label or goto is present in the dead code, it is not eliminated.
This restriction can be removed once label/goto checking occurs
much earlier in the compiler. In practice, it probably doesn't
matter much.

Updates golang#19699
Fixes golang#19705

name       old alloc/op      new alloc/op      delta
Template        39.2MB ± 0%       39.3MB ± 0%  +0.28%  (p=0.008 n=5+5)
Unicode         29.8MB ± 0%       29.8MB ± 0%    ~     (p=1.000 n=5+5)
GoTypes          113MB ± 0%        113MB ± 0%  -0.55%  (p=0.008 n=5+5)
SSA             1.25GB ± 0%       1.25GB ± 0%  +0.02%  (p=0.008 n=5+5)
Flate           25.3MB ± 0%       25.3MB ± 0%  -0.24%  (p=0.032 n=5+5)
GoParser        31.7MB ± 0%       31.8MB ± 0%  +0.31%  (p=0.008 n=5+5)
Reflect         78.2MB ± 0%       78.3MB ± 0%    ~     (p=0.421 n=5+5)
Tar             26.6MB ± 0%       26.7MB ± 0%  +0.21%  (p=0.008 n=5+5)
XML             42.2MB ± 0%       42.2MB ± 0%    ~     (p=0.056 n=5+5)

name       old allocs/op     new allocs/op     delta
Template          385k ± 0%         387k ± 0%  +0.51%  (p=0.016 n=5+5)
Unicode           321k ± 0%         321k ± 0%    ~     (p=1.000 n=5+5)
GoTypes          1.14M ± 0%        1.14M ± 0%    ~     (p=1.000 n=5+5)
SSA              9.71M ± 0%        9.72M ± 0%  +0.10%  (p=0.008 n=5+5)
Flate             234k ± 1%         234k ± 1%    ~     (p=0.690 n=5+5)
GoParser          315k ± 0%         317k ± 0%  +0.71%  (p=0.008 n=5+5)
Reflect           980k ± 0%         983k ± 0%  +0.30%  (p=0.032 n=5+5)
Tar               251k ± 0%         252k ± 0%  +0.55%  (p=0.016 n=5+5)
XML               392k ± 0%         393k ± 0%  +0.30%  (p=0.008 n=5+5)

Change-Id: Ia10ff4bbf5c6eae782582cc9cbc9785494d4fb83

@gopherbot gopherbot closed this in dffe5ac Apr 18, 2017

@golang golang locked and limited conversation to collaborators Apr 18, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.