From: Matthew Dempsky Date: Thu, 17 Aug 2023 05:57:12 +0000 (-0700) Subject: cmd/compile: enable zero-copy string->[]byte conversions X-Git-Tag: go1.22rc1~1231 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=c8adb3004ff05654d98041c76c68b64e1d479782;p=gostls13.git cmd/compile: enable zero-copy string->[]byte conversions This CL enables the latent support for string->[]byte conversions added go.dev/cl/520259. One catch is that we need to make sure []byte("") evaluates to a non-nil slice, even if "" is (nil, 0). This CL addresses that by adding a "ptr != nil" check for OSTR2BYTESTMP, unless the NonNil flag is set. The existing uses of OSTR2BYTESTMP (which aren't concerned about []byte("") evaluating to nil) are updated to set this flag. Fixes #2205. Change-Id: I35a9cb16c164cd86156b7560915aba5108d8b523 Reviewed-on: https://go-review.googlesource.com/c/go/+/520395 Run-TryBot: Matthew Dempsky TryBot-Result: Gopher Robot Auto-Submit: Matthew Dempsky Reviewed-by: Cuong Manh Le Reviewed-by: Dmitri Shuralyov --- diff --git a/src/cmd/compile/internal/escape/escape.go b/src/cmd/compile/internal/escape/escape.go index 2882f9fda3..5f5dab31f7 100644 --- a/src/cmd/compile/internal/escape/escape.go +++ b/src/cmd/compile/internal/escape/escape.go @@ -345,16 +345,11 @@ func (b *batch) finish(fns []*ir.Func) { // If the result of a string->[]byte conversion is never mutated, // then it can simply reuse the string's memory directly. - // - // TODO(mdempsky): Enable in a subsequent CL. We need to ensure - // []byte("") evaluates to []byte{}, not []byte(nil). - if false { - if n, ok := n.(*ir.ConvExpr); ok && n.Op() == ir.OSTR2BYTES && !loc.hasAttr(attrMutates) { - if base.Flag.LowerM >= 1 { - base.WarnfAt(n.Pos(), "zero-copy string->[]byte conversion") - } - n.SetOp(ir.OSTR2BYTESTMP) + if n, ok := n.(*ir.ConvExpr); ok && n.Op() == ir.OSTR2BYTES && !loc.hasAttr(attrMutates) { + if base.Flag.LowerM >= 1 { + base.WarnfAt(n.Pos(), "zero-copy string->[]byte conversion") } + n.SetOp(ir.OSTR2BYTESTMP) } } } diff --git a/src/cmd/compile/internal/ssagen/ssa.go b/src/cmd/compile/internal/ssagen/ssa.go index 25e93b531d..c3a47ac1d0 100644 --- a/src/cmd/compile/internal/ssagen/ssa.go +++ b/src/cmd/compile/internal/ssagen/ssa.go @@ -2659,6 +2659,14 @@ func (s *state) exprCheckPtr(n ir.Node, checkPtrOK bool) *ssa.Value { n := n.(*ir.ConvExpr) str := s.expr(n.X) ptr := s.newValue1(ssa.OpStringPtr, s.f.Config.Types.BytePtr, str) + if !n.NonNil() { + // We need to ensure []byte("") evaluates to []byte{}, and not []byte(nil). + // + // TODO(mdempsky): Investigate using "len != 0" instead of "ptr != nil". + cond := s.newValue2(ssa.OpNeqPtr, types.Types[types.TBOOL], ptr, s.constNil(ptr.Type)) + zerobase := s.newValue1A(ssa.OpAddr, ptr.Type, ir.Syms.Zerobase, s.sb) + ptr = s.ternary(cond, ptr, zerobase) + } len := s.newValue1(ssa.OpStringLen, types.Types[types.TINT], str) return s.newValue3(ssa.OpSliceMake, n.Type(), ptr, len, len) case ir.OCFUNC: diff --git a/src/cmd/compile/internal/walk/order.go b/src/cmd/compile/internal/walk/order.go index 3e3bda15e7..c38477f33e 100644 --- a/src/cmd/compile/internal/walk/order.go +++ b/src/cmd/compile/internal/walk/order.go @@ -815,8 +815,14 @@ func (o *orderState) stmt(n ir.Node) { // Mark []byte(str) range expression to reuse string backing storage. // It is safe because the storage cannot be mutated. n := n.(*ir.RangeStmt) - if n.X.Op() == ir.OSTR2BYTES { - n.X.(*ir.ConvExpr).SetOp(ir.OSTR2BYTESTMP) + if x, ok := n.X.(*ir.ConvExpr); ok { + switch x.Op() { + case ir.OSTR2BYTES: + x.SetOp(ir.OSTR2BYTESTMP) + fallthrough + case ir.OSTR2BYTESTMP: + x.MarkNonNil() // "range []byte(nil)" is fine + } } t := o.markTemp() diff --git a/src/cmd/compile/internal/walk/switch.go b/src/cmd/compile/internal/walk/switch.go index 3af457b8c0..f59ae33f51 100644 --- a/src/cmd/compile/internal/walk/switch.go +++ b/src/cmd/compile/internal/walk/switch.go @@ -736,6 +736,7 @@ func stringSearch(expr ir.Node, cc []exprClause, out *ir.Nodes) { // Convert expr to a []int8 slice := ir.NewConvExpr(base.Pos, ir.OSTR2BYTESTMP, types.NewSlice(types.Types[types.TINT8]), expr) slice.SetTypecheck(1) // legacy typechecker doesn't handle this op + slice.MarkNonNil() // Load the byte we're splitting on. load := ir.NewIndexExpr(base.Pos, slice, ir.NewInt(base.Pos, int64(bestIdx))) // Compare with the value we're splitting on. diff --git a/test/escape2.go b/test/escape2.go index e3e5904cde..99f85914a3 100644 --- a/test/escape2.go +++ b/test/escape2.go @@ -1729,7 +1729,7 @@ func intstring2() { func stringtoslicebyte0() { s := "foo" - x := []byte(s) // ERROR "\(\[\]byte\)\(s\) does not escape$" + x := []byte(s) // ERROR "\(\[\]byte\)\(s\) does not escape$" "zero-copy string->\[\]byte conversion" _ = x } diff --git a/test/escape2n.go b/test/escape2n.go index 57cc1a0163..350be65202 100644 --- a/test/escape2n.go +++ b/test/escape2n.go @@ -1729,7 +1729,7 @@ func intstring2() { func stringtoslicebyte0() { s := "foo" - x := []byte(s) // ERROR "\(\[\]byte\)\(s\) does not escape$" + x := []byte(s) // ERROR "\(\[\]byte\)\(s\) does not escape$" "zero-copy string->\[\]byte conversion" _ = x }