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

Jump to bottom

cmd/compile: internal compiler error: cannot inline function containing closure #19705

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

cmd/compile: internal compiler error: cannot inline function containing closure #19705

ALTree opened this issue Mar 24, 2017 · 4 comments
Labels
FrozenDueToAge
Milestone
Go1.9

Comments

@ALTree
Copy link
Member

ALTree 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
Copy link
Contributor

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

@ALTree
Copy link
Member Author

ALTree commented Mar 25, 2017

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

@josharian
Copy link
Contributor

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
Copy link

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

josharian added a commit to josharian/go that referenced this issue Apr 11, 2017
@josharian
cmd/compile: eliminate dead code in if statements after typechecking
cbf91d8 
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
@josharian josharian mentioned this issue May 2, 2017
Closed
@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.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
Go1.9
Development

No branches or pull requests
3 participants
@josharian @ALTree @gopherbot