From f4936d09fd5a1fff890d63ee2ab9543243dc4da6 Mon Sep 17 00:00:00 2001 From: Matthew Dempsky Date: Mon, 14 Sep 2020 12:56:37 -0700 Subject: cmd/compile: call fninit earlier This allows the global initializers function to go through normal mid-end optimizations (e.g., inlining, escape analysis) like any other function. Updates #33485. Change-Id: I9bcfe98b8628d1aca09b4c238d8d3b74c69010a5 Reviewed-on: https://go-review.googlesource.com/c/go/+/254839 Reviewed-by: Keith Randall Trust: Matthew Dempsky --- test/inline.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'test/inline.go') diff --git a/test/inline.go b/test/inline.go index 0b3ad55d46..1c5c1bc8d3 100644 --- a/test/inline.go +++ b/test/inline.go @@ -50,7 +50,7 @@ func j(x int) int { // ERROR "can inline j" } } -var somethingWrong error = errors.New("something went wrong") +var somethingWrong error = errors.New("something went wrong") // ERROR "can inline init" "inlining call to errors.New" "errors.errorString.* escapes to heap" // local closures can be inlined func l(x, y int) (int, int, error) { -- cgit v1.3 From bae9cf651796db898b1e4bd77a1a47c5f2d7b04d Mon Sep 17 00:00:00 2001 From: Matthew Dempsky Date: Mon, 14 Sep 2020 19:24:55 -0700 Subject: test: fix inline.go to pass linux-amd64-noopt Updates #33485. Change-Id: I3330860cdff1e9797466a7630bcdb7792c465b06 Reviewed-on: https://go-review.googlesource.com/c/go/+/254938 Run-TryBot: Matthew Dempsky Reviewed-by: Cuong Manh Le TryBot-Result: Go Bot Trust: Matthew Dempsky --- test/inline.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'test/inline.go') diff --git a/test/inline.go b/test/inline.go index 1c5c1bc8d3..3edcf2edfd 100644 --- a/test/inline.go +++ b/test/inline.go @@ -10,7 +10,6 @@ package foo import ( - "errors" "runtime" "unsafe" ) @@ -50,7 +49,7 @@ func j(x int) int { // ERROR "can inline j" } } -var somethingWrong error = errors.New("something went wrong") // ERROR "can inline init" "inlining call to errors.New" "errors.errorString.* escapes to heap" +var somethingWrong error // local closures can be inlined func l(x, y int) (int, int, error) { -- cgit v1.3 From 497ea0610ea3757c6171cae3a85627459b572e5d Mon Sep 17 00:00:00 2001 From: Matthew Dempsky Date: Mon, 21 Sep 2020 20:20:00 -0700 Subject: cmd/compile: allow inlining of "for" loops We already allow inlining "if" and "goto" statements, so we might as well allow "for" loops too. The majority of frontend support is already there too. The critical missing feature at the moment is that inlining doesn't properly reassociate OLABEL nodes with their control statement (e.g., OFOR) after inlining. This eventually causes SSA construction to fail. As a workaround, this CL only enables inlining for unlabeled "for" loops. It's left to a (yet unplanned) future CL to add support for labeled "for" loops. The increased opportunity for inlining leads to a small growth in binary size. For example: $ size go.old go.new text data bss dec hex filename 9740163 320064 230656 10290883 9d06c3 go.old 9793399 320064 230656 10344119 9dd6b7 go.new Updates #14768. Fixes #41474. Change-Id: I827db0b2b9d9fa2934db05caf6baa463f0cd032a Reviewed-on: https://go-review.googlesource.com/c/go/+/256459 Run-TryBot: Matthew Dempsky TryBot-Result: Go Bot Trust: Matthew Dempsky Reviewed-by: David Chase Reviewed-by: Cuong Manh Le --- src/cmd/compile/internal/gc/inl.go | 18 ++++++++++++++---- test/closure3.dir/main.go | 3 +-- test/inline.go | 23 +++++++++++++++++++++++ 3 files changed, 38 insertions(+), 6 deletions(-) (limited to 'test/inline.go') diff --git a/src/cmd/compile/internal/gc/inl.go b/src/cmd/compile/internal/gc/inl.go index cac51685df..8630560a9a 100644 --- a/src/cmd/compile/internal/gc/inl.go +++ b/src/cmd/compile/internal/gc/inl.go @@ -385,14 +385,11 @@ func (v *hairyVisitor) visit(n *Node) bool { case OCLOSURE, OCALLPART, ORANGE, - OFOR, - OFORUNTIL, OSELECT, OTYPESW, OGO, ODEFER, ODCLTYPE, // can't print yet - OBREAK, ORETJMP: v.reason = "unhandled op " + n.Op.String() return true @@ -400,10 +397,23 @@ func (v *hairyVisitor) visit(n *Node) bool { case OAPPEND: v.budget -= inlineExtraAppendCost - case ODCLCONST, OEMPTY, OFALL, OLABEL: + case ODCLCONST, OEMPTY, OFALL: // These nodes don't produce code; omit from inlining budget. return false + case OLABEL: + // TODO(mdempsky): Add support for inlining labeled control statements. + if n.labeledControl() != nil { + v.reason = "labeled control" + return true + } + + case OBREAK, OCONTINUE: + if n.Sym != nil { + // Should have short-circuited due to labeledControl above. + Fatalf("unexpected labeled break/continue: %v", n) + } + case OIF: if Isconst(n.Left, CTBOOL) { // This if and the condition cost nothing. diff --git a/test/closure3.dir/main.go b/test/closure3.dir/main.go index 3ec90139a3..5694673f1e 100644 --- a/test/closure3.dir/main.go +++ b/test/closure3.dir/main.go @@ -238,8 +238,7 @@ func main() { if c != 4 { ppanic("c != 4") } - for i := 0; i < 10; i++ { // prevent inlining - } + recover() // prevent inlining }() }() if c != 4 { diff --git a/test/inline.go b/test/inline.go index 3edcf2edfd..2f6fc0fe88 100644 --- a/test/inline.go +++ b/test/inline.go @@ -197,3 +197,26 @@ func gg(x int) { // ERROR "can inline gg" func hh(x int) { // ERROR "can inline hh" ff(x - 1) // ERROR "inlining call to ff" // ERROR "inlining call to gg" } + +// Issue #14768 - make sure we can inline for loops. +func for1(fn func() bool) { // ERROR "can inline for1" "fn does not escape" + for { + if fn() { + break + } else { + continue + } + } +} + +// BAD: for2 should be inlineable too. +func for2(fn func() bool) { // ERROR "fn does not escape" +Loop: + for { + if fn() { + break Loop + } else { + continue Loop + } + } +} -- cgit v1.3 From 1bcf6beec53ae811490fcd0ac29328b12b53702c Mon Sep 17 00:00:00 2001 From: Matthew Dempsky Date: Tue, 22 Sep 2020 02:19:14 -0700 Subject: cmd/compile: use staticValue for inlining logic This CL replaces the ad hoc and duplicated logic for detecting inlinable calls with a single "inlCallee" function, which uses the "staticValue" helper function introduced in an earlier commit. Updates #41474. Change-Id: I103d4091b10366fce1344ef2501222b7df68f21d Reviewed-on: https://go-review.googlesource.com/c/go/+/256460 Reviewed-by: David Chase Reviewed-by: Cuong Manh Le Trust: Matthew Dempsky --- src/cmd/compile/internal/gc/inl.go | 92 +++++++------------------- src/cmd/compile/internal/logopt/logopt_test.go | 1 - test/inline.go | 9 +++ 3 files changed, 34 insertions(+), 68 deletions(-) (limited to 'test/inline.go') diff --git a/src/cmd/compile/internal/gc/inl.go b/src/cmd/compile/internal/gc/inl.go index 8630560a9a..ba12cf40b5 100644 --- a/src/cmd/compile/internal/gc/inl.go +++ b/src/cmd/compile/internal/gc/inl.go @@ -325,18 +325,10 @@ func (v *hairyVisitor) visit(n *Node) bool { break } - if fn := n.Left.Func; fn != nil && fn.Inl != nil { - v.budget -= fn.Inl.Cost + if fn := inlCallee(n.Left); fn != nil && fn.Func.Inl != nil { + v.budget -= fn.Func.Inl.Cost break } - if n.Left.isMethodExpression() { - if d := asNode(n.Left.Sym.Def); d != nil && d.Func.Inl != nil { - v.budget -= d.Func.Inl.Cost - break - } - } - // TODO(mdempsky): Budget for OCLOSURE calls if we - // ever allow that. See #15561 and #23093. // Call cost for non-leaf inlining. v.budget -= v.extraCallCost @@ -679,53 +671,11 @@ func inlnode(n *Node, maxCost int32, inlMap map[*Node]bool) *Node { if Debug['m'] > 3 { fmt.Printf("%v:call to func %+v\n", n.Line(), n.Left) } - if n.Left.Func != nil && n.Left.Func.Inl != nil && !isIntrinsicCall(n) { // normal case - n = mkinlcall(n, n.Left, maxCost, inlMap) - } else if n.Left.isMethodExpression() && asNode(n.Left.Sym.Def) != nil { - n = mkinlcall(n, asNode(n.Left.Sym.Def), maxCost, inlMap) - } else if n.Left.Op == OCLOSURE { - if f := inlinableClosure(n.Left); f != nil { - n = mkinlcall(n, f, maxCost, inlMap) - } - } else if n.Left.Op == ONAME && n.Left.Name != nil && n.Left.Name.Defn != nil { - if d := n.Left.Name.Defn; d.Op == OAS && d.Right.Op == OCLOSURE { - if f := inlinableClosure(d.Right); f != nil { - // NB: this check is necessary to prevent indirect re-assignment of the variable - // having the address taken after the invocation or only used for reads is actually fine - // but we have no easy way to distinguish the safe cases - if d.Left.Name.Addrtaken() { - if Debug['m'] > 1 { - fmt.Printf("%v: cannot inline escaping closure variable %v\n", n.Line(), n.Left) - } - if logopt.Enabled() { - logopt.LogOpt(n.Pos, "cannotInlineCall", "inline", Curfn.funcname(), - fmt.Sprintf("%v cannot be inlined (escaping closure variable)", n.Left)) - } - break - } - - // ensure the variable is never re-assigned - if unsafe, a := reassigned(n.Left); unsafe { - if Debug['m'] > 1 { - if a != nil { - fmt.Printf("%v: cannot inline re-assigned closure variable at %v: %v\n", n.Line(), a.Line(), a) - if logopt.Enabled() { - logopt.LogOpt(n.Pos, "cannotInlineCall", "inline", Curfn.funcname(), - fmt.Sprintf("%v cannot be inlined (re-assigned closure variable)", a)) - } - } else { - fmt.Printf("%v: cannot inline global closure variable %v\n", n.Line(), n.Left) - if logopt.Enabled() { - logopt.LogOpt(n.Pos, "cannotInlineCall", "inline", Curfn.funcname(), - fmt.Sprintf("%v cannot be inlined (global closure variable)", n.Left)) - } - } - } - break - } - n = mkinlcall(n, f, maxCost, inlMap) - } - } + if isIntrinsicCall(n) { + break + } + if fn := inlCallee(n.Left); fn != nil && fn.Func.Inl != nil { + n = mkinlcall(n, fn, maxCost, inlMap) } case OCALLMETH: @@ -749,16 +699,22 @@ func inlnode(n *Node, maxCost int32, inlMap map[*Node]bool) *Node { return n } -// inlinableClosure takes an OCLOSURE node and follows linkage to the matching ONAME with -// the inlinable body. Returns nil if the function is not inlinable. -func inlinableClosure(n *Node) *Node { - c := n.Func.Closure - caninl(c) - f := c.Func.Nname - if f == nil || f.Func.Inl == nil { - return nil +// inlCallee takes a function-typed expression and returns the underlying function ONAME +// that it refers to if statically known. Otherwise, it returns nil. +func inlCallee(fn *Node) *Node { + fn = staticValue(fn) + switch { + case fn.Op == ONAME && fn.Class() == PFUNC: + if fn.isMethodExpression() { + return asNode(fn.Sym.Def) + } + return fn + case fn.Op == OCLOSURE: + c := fn.Func.Closure + caninl(c) + return c.Func.Nname } - return f + return nil } func staticValue(n *Node) *Node { @@ -771,7 +727,9 @@ func staticValue(n *Node) *Node { } } -// staticValue1 implements a simple SSA-like optimization. +// staticValue1 implements a simple SSA-like optimization. If n is a local variable +// that is initialized and never reassigned, staticValue1 returns the initializer +// expression. Otherwise, it returns nil. func staticValue1(n *Node) *Node { if n.Op != ONAME || n.Class() != PAUTO || n.Name.Addrtaken() { return nil diff --git a/src/cmd/compile/internal/logopt/logopt_test.go b/src/cmd/compile/internal/logopt/logopt_test.go index fb71e142e3..fca85c10fb 100644 --- a/src/cmd/compile/internal/logopt/logopt_test.go +++ b/src/cmd/compile/internal/logopt/logopt_test.go @@ -208,7 +208,6 @@ func s15a8(x *[15]int64) [15]int64 { `"relatedInformation":[{"location":{"uri":"file://tmpdir/file.go","range":{"start":{"line":4,"character":11},"end":{"line":4,"character":11}}},"message":"inlineLoc"}]}`) want(t, slogged, `{"range":{"start":{"line":11,"character":6},"end":{"line":11,"character":6}},"severity":3,"code":"isInBounds","source":"go compiler","message":""}`) want(t, slogged, `{"range":{"start":{"line":7,"character":6},"end":{"line":7,"character":6}},"severity":3,"code":"canInlineFunction","source":"go compiler","message":"cost: 35"}`) - want(t, slogged, `{"range":{"start":{"line":21,"character":21},"end":{"line":21,"character":21}},"severity":3,"code":"cannotInlineCall","source":"go compiler","message":"foo cannot be inlined (escaping closure variable)"}`) // escape analysis explanation want(t, slogged, `{"range":{"start":{"line":7,"character":13},"end":{"line":7,"character":13}},"severity":3,"code":"leak","source":"go compiler","message":"parameter z leaks to ~r2 with derefs=0",`+ `"relatedInformation":[`+ diff --git a/test/inline.go b/test/inline.go index 2f6fc0fe88..0e41873de4 100644 --- a/test/inline.go +++ b/test/inline.go @@ -49,6 +49,12 @@ func j(x int) int { // ERROR "can inline j" } } +func _() int { // ERROR "can inline _" + tmp1 := h + tmp2 := tmp1 + return tmp2(0) // ERROR "inlining call to h" +} + var somethingWrong error // local closures can be inlined @@ -58,6 +64,9 @@ func l(x, y int) (int, int, error) { } if x == y { e(somethingWrong) // ERROR "inlining call to l.func1" + } else { + f := e + f(nil) // ERROR "inlining call to l.func1" } return y, x, nil } -- cgit v1.3 From 8fe372c7b36b4d078c871a26e10b427c41275ecd Mon Sep 17 00:00:00 2001 From: Dan Scales Date: Mon, 19 Oct 2020 13:09:55 -0700 Subject: cmd/compile: allowing inlining of functions with OCALLPART OCALLPART is exported in its original form, which is as an OXDOT. The body of the method value wrapper created in makepartialcall() was not being typechecked, and that was causing a problem during escape analysis, so I added code to typecheck the body. The go executable got slightly bigger with this change (13598111 -> 13598905), because of extra exported methods with OCALLPART (I believe), while the text size got slightly smaller (9686964 -> 9686643). This is mainly part of the work to make sure all function bodies can be exported (for purposes of generics), but might as well fix the OCALLPART inlining bug as well. Fixes #18493 Change-Id: If7aa055ff78ed7a6330c6a1e22f836ec567d04fd Reviewed-on: https://go-review.googlesource.com/c/go/+/263620 Run-TryBot: Dan Scales TryBot-Result: Go Bot Reviewed-by: Keith Randall Reviewed-by: Matthew Dempsky --- src/cmd/compile/internal/gc/closure.go | 6 ++++++ src/cmd/compile/internal/gc/iexport.go | 9 +++++++-- src/cmd/compile/internal/gc/iimport.go | 2 +- src/cmd/compile/internal/gc/inl.go | 6 ++++-- test/inline.go | 17 +++++++++++++++++ 5 files changed, 35 insertions(+), 5 deletions(-) (limited to 'test/inline.go') diff --git a/src/cmd/compile/internal/gc/closure.go b/src/cmd/compile/internal/gc/closure.go index 250be38e5b..5d1012111f 100644 --- a/src/cmd/compile/internal/gc/closure.go +++ b/src/cmd/compile/internal/gc/closure.go @@ -434,6 +434,8 @@ func typecheckpartialcall(fn *Node, sym *types.Sym) { fn.Type = xfunc.Type } +// makepartialcall returns a DCLFUNC node representing the wrapper function (*-fm) needed +// for partial calls. func makepartialcall(fn *Node, t0 *types.Type, meth *types.Sym) *Node { rcvrtype := fn.Left.Type sym := methodSymSuffix(rcvrtype, meth, "-fm") @@ -500,6 +502,10 @@ func makepartialcall(fn *Node, t0 *types.Type, meth *types.Sym) *Node { funcbody() xfunc = typecheck(xfunc, ctxStmt) + // Need to typecheck the body of the just-generated wrapper. + // typecheckslice() requires that Curfn is set when processing an ORETURN. + Curfn = xfunc + typecheckslice(xfunc.Nbody.Slice(), ctxStmt) sym.Def = asTypesNode(xfunc) xtop = append(xtop, xfunc) Curfn = savecurfn diff --git a/src/cmd/compile/internal/gc/iexport.go b/src/cmd/compile/internal/gc/iexport.go index df08a4a6c2..9bc1f64600 100644 --- a/src/cmd/compile/internal/gc/iexport.go +++ b/src/cmd/compile/internal/gc/iexport.go @@ -1266,8 +1266,13 @@ func (w *exportWriter) expr(n *Node) { // case OSTRUCTKEY: // unreachable - handled in case OSTRUCTLIT by elemList - // case OCALLPART: - // unimplemented - handled by default case + case OCALLPART: + // An OCALLPART is an OXDOT before type checking. + w.op(OXDOT) + w.pos(n.Pos) + w.expr(n.Left) + // Right node should be ONAME + w.selector(n.Right.Sym) case OXDOT, ODOT, ODOTPTR, ODOTINTER, ODOTMETH: w.op(OXDOT) diff --git a/src/cmd/compile/internal/gc/iimport.go b/src/cmd/compile/internal/gc/iimport.go index 5f107eeec7..107e96cc6a 100644 --- a/src/cmd/compile/internal/gc/iimport.go +++ b/src/cmd/compile/internal/gc/iimport.go @@ -866,7 +866,7 @@ func (r *importReader) node() *Node { // unreachable - handled in case OSTRUCTLIT by elemList // case OCALLPART: - // unimplemented + // unreachable - mapped to case OXDOT below by exporter // case OXDOT, ODOT, ODOTPTR, ODOTINTER, ODOTMETH: // unreachable - mapped to case OXDOT below by exporter diff --git a/src/cmd/compile/internal/gc/inl.go b/src/cmd/compile/internal/gc/inl.go index ba12cf40b5..55a14d378e 100644 --- a/src/cmd/compile/internal/gc/inl.go +++ b/src/cmd/compile/internal/gc/inl.go @@ -374,8 +374,10 @@ func (v *hairyVisitor) visit(n *Node) bool { v.reason = "call to recover" return true + case OCALLPART: + // OCALLPART is inlineable, but no extra cost to the budget + case OCLOSURE, - OCALLPART, ORANGE, OSELECT, OTYPESW, @@ -454,7 +456,7 @@ func inlcopy(n *Node) *Node { } m := n.copy() - if m.Func != nil { + if n.Op != OCALLPART && m.Func != nil { Fatalf("unexpected Func: %v", m) } m.Left = inlcopy(n.Left) diff --git a/test/inline.go b/test/inline.go index 0e41873de4..9b75bc5065 100644 --- a/test/inline.go +++ b/test/inline.go @@ -229,3 +229,20 @@ Loop: } } } + +// Issue #18493 - make sure we can do inlining of functions with a method value +type T1 struct{} + +func (a T1) meth(val int) int { // ERROR "can inline T1.meth" "inlining call to T1.meth" + return val + 5 +} + +func getMeth(t1 T1) func(int) int { // ERROR "can inline getMeth" + return t1.meth // ERROR "t1.meth escapes to heap" +} + +func ii() { // ERROR "can inline ii" + var t1 T1 + f := getMeth(t1) // ERROR "inlining call to getMeth" "t1.meth does not escape" + _ = f(3) +} -- cgit v1.3 From 4fb429138881e3fe171e4c2e958ed0da26ddfd9c Mon Sep 17 00:00:00 2001 From: Branden J Brown Date: Sun, 25 Oct 2020 14:26:19 -0400 Subject: cmd/compile: inline functions evaluated in go and defer statements The inlining pass previously bailed upon encountering a go or defer statement, so it would not inline functions e.g. used to provide arguments to the deferred function. This change preserves the behavior of not inlining the deferred function itself, but it allows the inlining walk to proceed into its arguments. Fixes #42194 Change-Id: I4e82029d8dcbe69019cc83ae63a4b29af45ec777 Reviewed-on: https://go-review.googlesource.com/c/go/+/264997 Run-TryBot: Cuong Manh Le TryBot-Result: Go Bot Reviewed-by: Cuong Manh Le Reviewed-by: Matthew Dempsky Trust: Cuong Manh Le --- src/cmd/compile/internal/gc/inl.go | 2 -- test/inline.go | 17 +++++++++++++++++ 2 files changed, 17 insertions(+), 2 deletions(-) (limited to 'test/inline.go') diff --git a/src/cmd/compile/internal/gc/inl.go b/src/cmd/compile/internal/gc/inl.go index a2fb00e132..137675aa20 100644 --- a/src/cmd/compile/internal/gc/inl.go +++ b/src/cmd/compile/internal/gc/inl.go @@ -574,13 +574,11 @@ func inlnode(n *Node, maxCost int32, inlMap map[*Node]bool) *Node { } switch n.Op { - // inhibit inlining of their argument case ODEFER, OGO: switch n.Left.Op { case OCALLFUNC, OCALLMETH: n.Left.SetNoInline(true) } - return n // TODO do them here (or earlier), // so escape analysis can avoid more heapmoves. diff --git a/test/inline.go b/test/inline.go index 9b75bc5065..470414f883 100644 --- a/test/inline.go +++ b/test/inline.go @@ -246,3 +246,20 @@ func ii() { // ERROR "can inline ii" f := getMeth(t1) // ERROR "inlining call to getMeth" "t1.meth does not escape" _ = f(3) } + +// Issue #42194 - make sure that functions evaluated in +// go and defer statements can be inlined. +func gd1(int) { + defer gd1(gd2()) // ERROR "inlining call to gd2" + defer gd3()() // ERROR "inlining call to gd3" + go gd1(gd2()) // ERROR "inlining call to gd2" + go gd3()() // ERROR "inlining call to gd3" +} + +func gd2() int { // ERROR "can inline gd2" + return 1 +} + +func gd3() func() { // ERROR "can inline gd3" + return ii +} -- cgit v1.3 From 5736eb0013cb8c9b67432c98b08f68e9f370810c Mon Sep 17 00:00:00 2001 From: Matthew Dempsky Date: Thu, 29 Oct 2020 01:13:16 -0700 Subject: cmd/compile: support inlining of type switches This CL adds support for inlining type switches, including exporting and importing them. Type switches are represented mostly the same as expression switches. However, if the type switch guard includes a short variable declaration, then there are two differences: (1) there's an ONONAME (in the OTYPESW's Left) to represent the overall pseudo declaration; and (2) there's an ONAME (in each OCASE's Rlist) to represent the per-case variables. For simplicity, this CL simply writes out each variable separately using iimport/iiexport's normal Vargen mechanism for disambiguating identically named variables within a function. This could be improved somewhat, but inlinable type switches are probably too uncommon to merit the complexity. While here, remove "case OCASE" from typecheck1. We only type check "case" clauses as part of a "select" or "switch" statement, never as standalone statements. Fixes #37837 Change-Id: I8f42f6c9afdd821d6202af4a6bf1dbcbba0ef424 Reviewed-on: https://go-review.googlesource.com/c/go/+/266203 Run-TryBot: Matthew Dempsky TryBot-Result: Go Bot Reviewed-by: Keith Randall Trust: Matthew Dempsky --- src/cmd/compile/internal/gc/iexport.go | 40 ++++++++++++++++++++++++++----- src/cmd/compile/internal/gc/iimport.go | 41 +++++++++++++++++++++++++------- src/cmd/compile/internal/gc/inl.go | 4 ---- src/cmd/compile/internal/gc/typecheck.go | 5 ---- test/fixedbugs/issue37837.dir/a.go | 33 +++++++++++++++++++++++++ test/fixedbugs/issue37837.dir/b.go | 32 +++++++++++++++++++++++++ test/fixedbugs/issue37837.go | 7 ++++++ test/inline.go | 3 +-- 8 files changed, 140 insertions(+), 25 deletions(-) create mode 100644 test/fixedbugs/issue37837.dir/a.go create mode 100644 test/fixedbugs/issue37837.dir/b.go create mode 100644 test/fixedbugs/issue37837.go (limited to 'test/inline.go') diff --git a/src/cmd/compile/internal/gc/iexport.go b/src/cmd/compile/internal/gc/iexport.go index 9bc1f64600..1f53d8ca7d 100644 --- a/src/cmd/compile/internal/gc/iexport.go +++ b/src/cmd/compile/internal/gc/iexport.go @@ -1138,13 +1138,10 @@ func (w *exportWriter) stmt(n *Node) { w.pos(n.Pos) w.stmtList(n.Ninit) w.exprsOrNil(n.Left, nil) - w.stmtList(n.List) + w.caseList(n) - case OCASE: - w.op(OCASE) - w.pos(n.Pos) - w.stmtList(n.List) - w.stmtList(n.Nbody) + // case OCASE: + // handled by caseList case OFALL: w.op(OFALL) @@ -1168,6 +1165,24 @@ func (w *exportWriter) stmt(n *Node) { } } +func (w *exportWriter) caseList(sw *Node) { + namedTypeSwitch := sw.Op == OSWITCH && sw.Left != nil && sw.Left.Op == OTYPESW && sw.Left.Left != nil + + cases := sw.List.Slice() + w.uint64(uint64(len(cases))) + for _, cas := range cases { + if cas.Op != OCASE { + Fatalf("expected OCASE, got %v", cas) + } + w.pos(cas.Pos) + w.stmtList(cas.List) + if namedTypeSwitch { + w.localName(cas.Rlist.First()) + } + w.stmtList(cas.Nbody) + } +} + func (w *exportWriter) exprList(list Nodes) { for _, n := range list.Slice() { w.expr(n) @@ -1232,6 +1247,19 @@ func (w *exportWriter) expr(n *Node) { w.op(OTYPE) w.typ(n.Type) + case OTYPESW: + w.op(OTYPESW) + w.pos(n.Pos) + var s *types.Sym + if n.Left != nil { + if n.Left.Op != ONONAME { + Fatalf("expected ONONAME, got %v", n.Left) + } + s = n.Left.Sym + } + w.localIdent(s, 0) // declared pseudo-variable, if any + w.exprsOrNil(n.Right, nil) + // case OTARRAY, OTMAP, OTCHAN, OTSTRUCT, OTINTER, OTFUNC: // should have been resolved by typechecking - handled by default case diff --git a/src/cmd/compile/internal/gc/iimport.go b/src/cmd/compile/internal/gc/iimport.go index 7f2b05f288..c0114d0e53 100644 --- a/src/cmd/compile/internal/gc/iimport.go +++ b/src/cmd/compile/internal/gc/iimport.go @@ -784,6 +784,28 @@ func (r *importReader) stmtList() []*Node { return list } +func (r *importReader) caseList(sw *Node) []*Node { + namedTypeSwitch := sw.Op == OSWITCH && sw.Left != nil && sw.Left.Op == OTYPESW && sw.Left.Left != nil + + cases := make([]*Node, r.uint64()) + for i := range cases { + cas := nodl(r.pos(), OCASE, nil, nil) + cas.List.Set(r.stmtList()) + if namedTypeSwitch { + // Note: per-case variables will have distinct, dotted + // names after import. That's okay: swt.go only needs + // Sym for diagnostics anyway. + caseVar := newnamel(cas.Pos, r.ident()) + declare(caseVar, dclcontext) + cas.Rlist.Set1(caseVar) + caseVar.Name.Defn = sw.Left + } + cas.Nbody.Set(r.stmtList()) + cases[i] = cas + } + return cases +} + func (r *importReader) exprList() []*Node { var list []*Node for { @@ -831,6 +853,14 @@ func (r *importReader) node() *Node { case OTYPE: return typenod(r.typ()) + case OTYPESW: + n := nodl(r.pos(), OTYPESW, nil, nil) + if s := r.ident(); s != nil { + n.Left = npos(n.Pos, newnoname(s)) + } + n.Right, _ = r.exprsOrNil() + return n + // case OTARRAY, OTMAP, OTCHAN, OTSTRUCT, OTINTER, OTFUNC: // unreachable - should have been resolved by typechecking @@ -1025,16 +1055,11 @@ func (r *importReader) node() *Node { n := nodl(r.pos(), op, nil, nil) n.Ninit.Set(r.stmtList()) n.Left, _ = r.exprsOrNil() - n.List.Set(r.stmtList()) + n.List.Set(r.caseList(n)) return n - case OCASE: - n := nodl(r.pos(), OCASE, nil, nil) - n.List.Set(r.exprList()) - // TODO(gri) eventually we must declare variables for type switch - // statements (type switch statements are not yet exported) - n.Nbody.Set(r.stmtList()) - return n + // case OCASE: + // handled by caseList case OFALL: n := nodl(r.pos(), OFALL, nil, nil) diff --git a/src/cmd/compile/internal/gc/inl.go b/src/cmd/compile/internal/gc/inl.go index 253036fea6..139572f652 100644 --- a/src/cmd/compile/internal/gc/inl.go +++ b/src/cmd/compile/internal/gc/inl.go @@ -392,13 +392,9 @@ func (v *hairyVisitor) visit(n *Node) bool { v.reason = "call to recover" return true - case OCALLPART: - // OCALLPART is inlineable, but no extra cost to the budget - case OCLOSURE, ORANGE, OSELECT, - OTYPESW, OGO, ODEFER, ODCLTYPE, // can't print yet diff --git a/src/cmd/compile/internal/gc/typecheck.go b/src/cmd/compile/internal/gc/typecheck.go index 8ebeaf1330..cbba5ff79c 100644 --- a/src/cmd/compile/internal/gc/typecheck.go +++ b/src/cmd/compile/internal/gc/typecheck.go @@ -2065,11 +2065,6 @@ func typecheck1(n *Node, top int) (res *Node) { n.Type = nil return n - case OCASE: - ok |= ctxStmt - typecheckslice(n.List.Slice(), ctxExpr) - typecheckslice(n.Nbody.Slice(), ctxStmt) - case ODCLFUNC: ok |= ctxStmt typecheckfunc(n) diff --git a/test/fixedbugs/issue37837.dir/a.go b/test/fixedbugs/issue37837.dir/a.go new file mode 100644 index 0000000000..49d830ffbc --- /dev/null +++ b/test/fixedbugs/issue37837.dir/a.go @@ -0,0 +1,33 @@ +// Copyright 2020 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package a + +func F(i interface{}) int { // ERROR "can inline F" "i does not escape" + switch i.(type) { + case nil: + return 0 + case int: + return 1 + case float64: + return 2 + default: + return 3 + } +} + +func G(i interface{}) interface{} { // ERROR "can inline G" "leaking param: i" + switch i := i.(type) { + case nil: // ERROR "moved to heap: i" + return &i + case int: // ERROR "moved to heap: i" + return &i + case float64: // ERROR "moved to heap: i" + return &i + case string, []byte: // ERROR "moved to heap: i" + return &i + default: // ERROR "moved to heap: i" + return &i + } +} diff --git a/test/fixedbugs/issue37837.dir/b.go b/test/fixedbugs/issue37837.dir/b.go new file mode 100644 index 0000000000..461f5c7a55 --- /dev/null +++ b/test/fixedbugs/issue37837.dir/b.go @@ -0,0 +1,32 @@ +// Copyright 2020 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package main + +import "./a" + +func main() { + // Test that inlined type switches without short variable + // declarations work correctly. + check(0, a.F(nil)) // ERROR "inlining call to a.F" + check(1, a.F(0)) // ERROR "inlining call to a.F" "does not escape" + check(2, a.F(0.0)) // ERROR "inlining call to a.F" "does not escape" + check(3, a.F("")) // ERROR "inlining call to a.F" "does not escape" + + // Test that inlined type switches with short variable + // declarations work correctly. + _ = a.G(nil).(*interface{}) // ERROR "inlining call to a.G" + _ = a.G(1).(*int) // ERROR "inlining call to a.G" "does not escape" + _ = a.G(2.0).(*float64) // ERROR "inlining call to a.G" "does not escape" + _ = (*a.G("").(*interface{})).(string) // ERROR "inlining call to a.G" "does not escape" + _ = (*a.G(([]byte)(nil)).(*interface{})).([]byte) // ERROR "inlining call to a.G" "does not escape" + _ = (*a.G(true).(*interface{})).(bool) // ERROR "inlining call to a.G" "does not escape" +} + +//go:noinline +func check(want, got int) { + if want != got { + println("want", want, "but got", got) + } +} diff --git a/test/fixedbugs/issue37837.go b/test/fixedbugs/issue37837.go new file mode 100644 index 0000000000..2e8abc5f05 --- /dev/null +++ b/test/fixedbugs/issue37837.go @@ -0,0 +1,7 @@ +// errorcheckandrundir -0 -m + +// Copyright 2020 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package ignored diff --git a/test/inline.go b/test/inline.go index 470414f883..d754f06e03 100644 --- a/test/inline.go +++ b/test/inline.go @@ -152,8 +152,7 @@ func switchBreak(x, y int) int { return n } -// can't currently inline functions with a type switch -func switchType(x interface{}) int { // ERROR "x does not escape" +func switchType(x interface{}) int { // ERROR "can inline switchType" "x does not escape" switch x.(type) { case int: return x.(int) -- cgit v1.3