From 3e4916fbf07335d88182b27f04821e4ce6be5f45 Mon Sep 17 00:00:00 2001 From: Mateusz Poliwczak Date: Wed, 12 Feb 2025 19:25:54 +0100 Subject: [PATCH 1/6] cmd/compile: determine static len and cap values of make() calls Change-Id: I70bd8d4961f12cb16cfd94650ac7ff8e4243163b --- src/cmd/compile/internal/escape/escape.go | 32 +++++++ test/escape_make_non_const.go | 107 ++++++++++++++++++++++ 2 files changed, 139 insertions(+) create mode 100644 test/escape_make_non_const.go diff --git a/src/cmd/compile/internal/escape/escape.go b/src/cmd/compile/internal/escape/escape.go index d6f0708a7f17ef..510412daed0523 100644 --- a/src/cmd/compile/internal/escape/escape.go +++ b/src/cmd/compile/internal/escape/escape.go @@ -6,6 +6,8 @@ package escape import ( "fmt" + "go/constant" + "go/token" "cmd/compile/internal/base" "cmd/compile/internal/ir" @@ -119,6 +121,36 @@ type escape struct { } func Funcs(all []*ir.Func) { + // Try to determine static values of make() calls, to avoid allocating them on the heap. + // We are doing this in escape analysis, so that it happens after inlining and devirtualization. + for _, v := range all { + ir.VisitList(v.Body, func(n ir.Node) { + if n, ok := n.(*ir.MakeExpr); ok { + if n.Cap != nil { + if s := ir.StaticValue(n.Cap); s.Op() == ir.OLITERAL { + if v, ok := s.(*ir.BasicLit); !ok || v.Val().Kind() != constant.Int { + base.Fatalf("unexpected BasicLit Kind") + } + n.Cap = s + } + } + if n.Len != nil { + if s := ir.StaticValue(n.Len); s.Op() == ir.OLITERAL { + len, ok := s.(*ir.BasicLit) + if !ok || len.Val().Kind() != constant.Int { + base.Fatalf("unexpected BasicLit Kind") + } + + cap, ok := n.Cap.(*ir.BasicLit) + if n.Cap == nil || (ok && constant.Compare(cap.Val(), token.GEQ, len.Val())) { + n.Len = s + } + } + } + } + }) + } + ir.VisitFuncsBottomUp(all, Batch) } diff --git a/test/escape_make_non_const.go b/test/escape_make_non_const.go new file mode 100644 index 00000000000000..0ea5ec6be74654 --- /dev/null +++ b/test/escape_make_non_const.go @@ -0,0 +1,107 @@ +// errorcheck -0 -m + +// Copyright 2025 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 escape + +const globalConstSize = 128 + +var globalVarSize = 128 + +//go:noinline +func testSlices() { + { + size := 128 + _ = make([]byte, size) // ERROR "does not escape" + } + + { + s := 128 + size := s + _ = make([]byte, size) // ERROR "does not escape" + } + + { + size := 128 + _ = make([]byte, 0, size) // ERROR "does not escape" + } + + { + s := 128 + size := s + _ = make([]byte, 0, size) // ERROR "does not escape" + } + + { + s1 := 128 + s2 := 256 + _ = make([]byte, s2, s1) // ERROR "does not escape" + } + + allocLen(256) // ERROR "does not escape" "inlining call" + allocCap(256) // ERROR "does not escape" "inlining call" + _ = newT(256) // ERROR "does not escape" "inlining call" + + { + size := globalConstSize + _ = make([]byte, size) // ERROR "does not escape" + } + + allocLen(globalConstSize) // ERROR "does not escape" "inlining call" + allocCap(globalConstSize) // ERROR "does not escape" "inlining call" + _ = newT(globalConstSize) // ERROR "does not escape" "inlining call" + + { + c := 128 + s := 256 + _ = make([]byte, s, c) // ERROR "make\(\[\]byte, s, 128\) does not escape" + } + + { + s := 256 + _ = make([]byte, s, globalConstSize) // ERROR "make\(\[\]byte, s, 128\) does not escape" + } + + { + _ = make([]byte, globalVarSize) // ERROR "escapes to heap" + } +} + +func allocLen(l int) []byte { // ERROR "can inline" + return make([]byte, l) // ERROR "escapes to heap" +} + +func allocCap(l int) []byte { // ERROR "can inline" + return make([]byte, 0, l) // ERROR "escapes to heap" +} + +type t struct { + s []byte +} + +func newT(l int) t { // ERROR "can inline" + return t{make([]byte, l)} // ERROR "make.*escapes to heap" +} + +//go:noinline +func testMaps() { + size := 128 + _ = make(map[string]int, size) // ERROR "does not escape" + + _ = allocMapLen(128) // ERROR "does not escape" "inlining call" + _ = newM(128) // ERROR "does not escape" "inlining call" +} + +func allocMapLen(l int) map[string]int { // ERROR "can inline" + return make(map[string]int, l) // ERROR "escapes to heap" +} + +type m struct { + m map[string]int +} + +func newM(l int) m { // ERROR "can inline" + return m{make(map[string]int, l)} // ERROR "make.*escapes to heap" +} From a6e512ca76d0fb036639335e51d94ffdf68a609c Mon Sep 17 00:00:00 2001 From: Mateusz Poliwczak Date: Wed, 12 Feb 2025 20:00:06 +0100 Subject: [PATCH 2/6] update tests Change-Id: If229448904be3f8fe4a013cba2f74856b0e79f60 --- test/escape_array.go | 4 ++-- test/escape_slice.go | 2 +- test/fixedbugs/issue41635.go | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/test/escape_array.go b/test/escape_array.go index 83062c9436e71d..1a1eb5e4aafb6f 100644 --- a/test/escape_array.go +++ b/test/escape_array.go @@ -123,7 +123,7 @@ func doesMakeSlice(x *string, y *string) { // ERROR "leaking param: x" "leaking func nonconstArray() { n := 32 - s1 := make([]int, n) // ERROR "make\(\[\]int, n\) escapes to heap" - s2 := make([]int, 0, n) // ERROR "make\(\[\]int, 0, n\) escapes to heap" + s1 := make([]int, n) // ERROR "make\(\[\]int, 32\) does not escape" + s2 := make([]int, 0, n) // ERROR "make\(\[\]int, 0, 32\) does not escape" _, _ = s1, s2 } diff --git a/test/escape_slice.go b/test/escape_slice.go index 65181e57d7edd3..9687722a4b2c9c 100644 --- a/test/escape_slice.go +++ b/test/escape_slice.go @@ -96,7 +96,7 @@ func slice10() []*int { func slice11() { i := 2 s := make([]int, 2, 3) // ERROR "make\(\[\]int, 2, 3\) does not escape" - s = make([]int, i, 3) // ERROR "make\(\[\]int, i, 3\) does not escape" + s = make([]int, i, 3) // ERROR "make\(\[\]int, 2, 3\) does not escape" s = make([]int, i, 1) // ERROR "make\(\[\]int, i, 1\) does not escape" _ = s } diff --git a/test/fixedbugs/issue41635.go b/test/fixedbugs/issue41635.go index 35c0034cdd40e0..ede8a4f2c9c64b 100644 --- a/test/fixedbugs/issue41635.go +++ b/test/fixedbugs/issue41635.go @@ -12,6 +12,6 @@ func f() { // ERROR "" _ = make([]byte, 100, 1<<17) // ERROR "too large for stack" "" _ = make([]byte, n, 1<<17) // ERROR "too large for stack" "" - _ = make([]byte, n) // ERROR "non-constant size" "" - _ = make([]byte, 100, m) // ERROR "non-constant size" "" + _ = make([]byte, n) // ERROR "does not escape" + _ = make([]byte, 100, m) // ERROR "does not escape" } From e42e793cd86ae343bddb30d7c54d1d29c7388d39 Mon Sep 17 00:00:00 2001 From: Mateusz Poliwczak Date: Wed, 12 Feb 2025 20:12:37 +0100 Subject: [PATCH 3/6] make sure that len and cap are >= 0 Change-Id: I687b2ac63a61b82561013b87f0b9d0c37c640719 --- src/cmd/compile/internal/escape/escape.go | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/cmd/compile/internal/escape/escape.go b/src/cmd/compile/internal/escape/escape.go index 510412daed0523..0c3ceba6daa533 100644 --- a/src/cmd/compile/internal/escape/escape.go +++ b/src/cmd/compile/internal/escape/escape.go @@ -128,10 +128,13 @@ func Funcs(all []*ir.Func) { if n, ok := n.(*ir.MakeExpr); ok { if n.Cap != nil { if s := ir.StaticValue(n.Cap); s.Op() == ir.OLITERAL { - if v, ok := s.(*ir.BasicLit); !ok || v.Val().Kind() != constant.Int { + cap, ok := s.(*ir.BasicLit) + if !ok || cap.Val().Kind() != constant.Int { base.Fatalf("unexpected BasicLit Kind") } - n.Cap = s + if constant.Compare(cap.Val(), token.GEQ, constant.MakeInt64(0)) { + n.Cap = s + } } } if n.Len != nil { @@ -141,9 +144,11 @@ func Funcs(all []*ir.Func) { base.Fatalf("unexpected BasicLit Kind") } - cap, ok := n.Cap.(*ir.BasicLit) - if n.Cap == nil || (ok && constant.Compare(cap.Val(), token.GEQ, len.Val())) { - n.Len = s + if constant.Compare(len.Val(), token.GEQ, constant.MakeInt64(0)) { + cap, ok := n.Cap.(*ir.BasicLit) + if n.Cap == nil || (ok && constant.Compare(cap.Val(), token.GEQ, len.Val())) { + n.Len = s + } } } } From 1bacc014a022e357d1d8ed07df71625f59fb8221 Mon Sep 17 00:00:00 2001 From: Mateusz Poliwczak Date: Thu, 13 Feb 2025 08:20:58 +0100 Subject: [PATCH 4/6] move to utils --- src/cmd/compile/internal/escape/escape.go | 37 ----------------------- src/cmd/compile/internal/escape/utils.go | 33 ++++++++++++++++++++ 2 files changed, 33 insertions(+), 37 deletions(-) diff --git a/src/cmd/compile/internal/escape/escape.go b/src/cmd/compile/internal/escape/escape.go index 0c3ceba6daa533..d6f0708a7f17ef 100644 --- a/src/cmd/compile/internal/escape/escape.go +++ b/src/cmd/compile/internal/escape/escape.go @@ -6,8 +6,6 @@ package escape import ( "fmt" - "go/constant" - "go/token" "cmd/compile/internal/base" "cmd/compile/internal/ir" @@ -121,41 +119,6 @@ type escape struct { } func Funcs(all []*ir.Func) { - // Try to determine static values of make() calls, to avoid allocating them on the heap. - // We are doing this in escape analysis, so that it happens after inlining and devirtualization. - for _, v := range all { - ir.VisitList(v.Body, func(n ir.Node) { - if n, ok := n.(*ir.MakeExpr); ok { - if n.Cap != nil { - if s := ir.StaticValue(n.Cap); s.Op() == ir.OLITERAL { - cap, ok := s.(*ir.BasicLit) - if !ok || cap.Val().Kind() != constant.Int { - base.Fatalf("unexpected BasicLit Kind") - } - if constant.Compare(cap.Val(), token.GEQ, constant.MakeInt64(0)) { - n.Cap = s - } - } - } - if n.Len != nil { - if s := ir.StaticValue(n.Len); s.Op() == ir.OLITERAL { - len, ok := s.(*ir.BasicLit) - if !ok || len.Val().Kind() != constant.Int { - base.Fatalf("unexpected BasicLit Kind") - } - - if constant.Compare(len.Val(), token.GEQ, constant.MakeInt64(0)) { - cap, ok := n.Cap.(*ir.BasicLit) - if n.Cap == nil || (ok && constant.Compare(cap.Val(), token.GEQ, len.Val())) { - n.Len = s - } - } - } - } - } - }) - } - ir.VisitFuncsBottomUp(all, Batch) } diff --git a/src/cmd/compile/internal/escape/utils.go b/src/cmd/compile/internal/escape/utils.go index bd1d2c22a2cfb8..c61b5ef068f91b 100644 --- a/src/cmd/compile/internal/escape/utils.go +++ b/src/cmd/compile/internal/escape/utils.go @@ -5,9 +5,12 @@ package escape import ( + "cmd/compile/internal/base" "cmd/compile/internal/ir" "cmd/compile/internal/typecheck" "cmd/compile/internal/types" + "go/constant" + "go/token" ) func isSliceSelfAssign(dst, src ir.Node) bool { @@ -206,6 +209,36 @@ func HeapAllocReason(n ir.Node) string { if n.Op() == ir.OMAKESLICE { n := n.(*ir.MakeExpr) + + // Try to determine static values of make() calls, to avoid allocating them on the heap. + // We are doing this in escape analysis, so that it happens after inlining and devirtualization. + if n.Cap != nil { + if s := ir.StaticValue(n.Cap); s.Op() == ir.OLITERAL { + cap, ok := s.(*ir.BasicLit) + if !ok || cap.Val().Kind() != constant.Int { + base.Fatalf("unexpected BasicLit Kind") + } + if constant.Compare(cap.Val(), token.GEQ, constant.MakeInt64(0)) { + n.Cap = s + } + } + } + if n.Len != nil { + if s := ir.StaticValue(n.Len); s.Op() == ir.OLITERAL { + len, ok := s.(*ir.BasicLit) + if !ok || len.Val().Kind() != constant.Int { + base.Fatalf("unexpected BasicLit Kind") + } + + if constant.Compare(len.Val(), token.GEQ, constant.MakeInt64(0)) { + cap, ok := n.Cap.(*ir.BasicLit) + if n.Cap == nil || (ok && constant.Compare(cap.Val(), token.GEQ, len.Val())) { + n.Len = s + } + } + } + } + r := n.Cap if r == nil { r = n.Len From e5847b26c32fdf2870f0bf7a70f737666517ba20 Mon Sep 17 00:00:00 2001 From: Mateusz Poliwczak Date: Mon, 17 Feb 2025 11:07:02 +0100 Subject: [PATCH 5/6] update, based on review comments Change-Id: I6175d154af235a31dd1c985827b99bdd7c0ac993 --- src/cmd/compile/internal/escape/utils.go | 9 ++------ test/escape_make_non_const.go | 27 ++++++++++++------------ 2 files changed, 16 insertions(+), 20 deletions(-) diff --git a/src/cmd/compile/internal/escape/utils.go b/src/cmd/compile/internal/escape/utils.go index c61b5ef068f91b..efe69ffc583ee2 100644 --- a/src/cmd/compile/internal/escape/utils.go +++ b/src/cmd/compile/internal/escape/utils.go @@ -222,19 +222,14 @@ func HeapAllocReason(n ir.Node) string { n.Cap = s } } - } - if n.Len != nil { + } else if n.Len != nil { if s := ir.StaticValue(n.Len); s.Op() == ir.OLITERAL { len, ok := s.(*ir.BasicLit) if !ok || len.Val().Kind() != constant.Int { base.Fatalf("unexpected BasicLit Kind") } - if constant.Compare(len.Val(), token.GEQ, constant.MakeInt64(0)) { - cap, ok := n.Cap.(*ir.BasicLit) - if n.Cap == nil || (ok && constant.Compare(cap.Val(), token.GEQ, len.Val())) { - n.Len = s - } + n.Len = s } } } diff --git a/test/escape_make_non_const.go b/test/escape_make_non_const.go index 0ea5ec6be74654..b5f5cb2e7172f6 100644 --- a/test/escape_make_non_const.go +++ b/test/escape_make_non_const.go @@ -14,44 +14,44 @@ var globalVarSize = 128 func testSlices() { { size := 128 - _ = make([]byte, size) // ERROR "does not escape" + _ = make([]byte, size) // ERROR "make\(\[\]byte, 128\) does not escape" } { s := 128 size := s - _ = make([]byte, size) // ERROR "does not escape" + _ = make([]byte, size) // ERROR "make\(\[\]byte, 128\) does not escape" } { size := 128 - _ = make([]byte, 0, size) // ERROR "does not escape" + _ = make([]byte, size) // ERROR "make\(\[\]byte, 128\) does not escape" } { s := 128 size := s - _ = make([]byte, 0, size) // ERROR "does not escape" + _ = make([]byte, size) // ERROR "make\(\[\]byte, 128\) does not escape" } { s1 := 128 s2 := 256 - _ = make([]byte, s2, s1) // ERROR "does not escape" + _ = make([]byte, s2, s1) // ERROR "make\(\[\]byte, s2, 128\) does not escape" } - allocLen(256) // ERROR "does not escape" "inlining call" - allocCap(256) // ERROR "does not escape" "inlining call" - _ = newT(256) // ERROR "does not escape" "inlining call" + allocLen(256) // ERROR "make\(\[\]byte, 256\) does not escape" "inlining call" + allocCap(256) // ERROR "make\(\[\]byte, 0, 256\) does not escape" "inlining call" + _ = newT(256) // ERROR "make\(\[\]byte, 256\) does not escape" "inlining call" { size := globalConstSize - _ = make([]byte, size) // ERROR "does not escape" + _ = make([]byte, size) // ERROR "make\(\[\]byte, 128\) does not escape" } - allocLen(globalConstSize) // ERROR "does not escape" "inlining call" - allocCap(globalConstSize) // ERROR "does not escape" "inlining call" - _ = newT(globalConstSize) // ERROR "does not escape" "inlining call" + allocLen(globalConstSize) // ERROR "make\(\[\]byte, 128\) does not escape" "inlining call" + allocCap(globalConstSize) // ERROR "make\(\[\]byte, 0, 128\) does not escape" "inlining call" + _ = newT(globalConstSize) // ERROR "make\(\[\]byte, 128\) does not escape" "inlining call" { c := 128 @@ -65,7 +65,8 @@ func testSlices() { } { - _ = make([]byte, globalVarSize) // ERROR "escapes to heap" + _ = make([]byte, globalVarSize) // ERROR "make\(\[\]byte, globalVarSize\) escapes to heap" + _ = make([]byte, globalVarSize, globalConstSize) // ERROR "make\(\[\]byte, globalVarSize, 128\) does not escape" } } From d78c1b4ca55fa53282e665009f689d0b013f1434 Mon Sep 17 00:00:00 2001 From: Mateusz Poliwczak Date: Mon, 17 Feb 2025 12:18:07 +0100 Subject: [PATCH 6/6] simplify, fix test Change-Id: I2f845692852ecc65db21ac2160123b72fdcf4892 --- src/cmd/compile/internal/escape/utils.go | 37 +++++++++--------------- test/escape_slice.go | 2 +- 2 files changed, 14 insertions(+), 25 deletions(-) diff --git a/src/cmd/compile/internal/escape/utils.go b/src/cmd/compile/internal/escape/utils.go index efe69ffc583ee2..d9cb9bdf8ec1bc 100644 --- a/src/cmd/compile/internal/escape/utils.go +++ b/src/cmd/compile/internal/escape/utils.go @@ -210,38 +210,27 @@ func HeapAllocReason(n ir.Node) string { if n.Op() == ir.OMAKESLICE { n := n.(*ir.MakeExpr) + r := &n.Cap + if n.Cap == nil { + r = &n.Len + } + // Try to determine static values of make() calls, to avoid allocating them on the heap. // We are doing this in escape analysis, so that it happens after inlining and devirtualization. - if n.Cap != nil { - if s := ir.StaticValue(n.Cap); s.Op() == ir.OLITERAL { - cap, ok := s.(*ir.BasicLit) - if !ok || cap.Val().Kind() != constant.Int { - base.Fatalf("unexpected BasicLit Kind") - } - if constant.Compare(cap.Val(), token.GEQ, constant.MakeInt64(0)) { - n.Cap = s - } + if s := ir.StaticValue(*r); s.Op() == ir.OLITERAL { + lit, ok := s.(*ir.BasicLit) + if !ok || lit.Val().Kind() != constant.Int { + base.Fatalf("unexpected BasicLit Kind") } - } else if n.Len != nil { - if s := ir.StaticValue(n.Len); s.Op() == ir.OLITERAL { - len, ok := s.(*ir.BasicLit) - if !ok || len.Val().Kind() != constant.Int { - base.Fatalf("unexpected BasicLit Kind") - } - if constant.Compare(len.Val(), token.GEQ, constant.MakeInt64(0)) { - n.Len = s - } + if constant.Compare(lit.Val(), token.GEQ, constant.MakeInt64(0)) { + *r = lit } } - r := n.Cap - if r == nil { - r = n.Len - } - if !ir.IsSmallIntConst(r) { + if !ir.IsSmallIntConst(*r) { return "non-constant size" } - if t := n.Type(); t.Elem().Size() != 0 && ir.Int64Val(r) > ir.MaxImplicitStackVarSize/t.Elem().Size() { + if t := n.Type(); t.Elem().Size() != 0 && ir.Int64Val(*r) > ir.MaxImplicitStackVarSize/t.Elem().Size() { return "too large for stack" } } diff --git a/test/escape_slice.go b/test/escape_slice.go index 9687722a4b2c9c..65181e57d7edd3 100644 --- a/test/escape_slice.go +++ b/test/escape_slice.go @@ -96,7 +96,7 @@ func slice10() []*int { func slice11() { i := 2 s := make([]int, 2, 3) // ERROR "make\(\[\]int, 2, 3\) does not escape" - s = make([]int, i, 3) // ERROR "make\(\[\]int, 2, 3\) does not escape" + s = make([]int, i, 3) // ERROR "make\(\[\]int, i, 3\) does not escape" s = make([]int, i, 1) // ERROR "make\(\[\]int, i, 1\) does not escape" _ = s }