Skip to content

Commit c8adb30

Browse files
mdempskygopherbot
authored andcommitted
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 <mdempsky@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Auto-Submit: Matthew Dempsky <mdempsky@google.com> Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
1 parent b805e18 commit c8adb30

File tree

6 files changed

+23
-13
lines changed

6 files changed

+23
-13
lines changed

src/cmd/compile/internal/escape/escape.go

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -345,16 +345,11 @@ func (b *batch) finish(fns []*ir.Func) {
345345

346346
// If the result of a string->[]byte conversion is never mutated,
347347
// then it can simply reuse the string's memory directly.
348-
//
349-
// TODO(mdempsky): Enable in a subsequent CL. We need to ensure
350-
// []byte("") evaluates to []byte{}, not []byte(nil).
351-
if false {
352-
if n, ok := n.(*ir.ConvExpr); ok && n.Op() == ir.OSTR2BYTES && !loc.hasAttr(attrMutates) {
353-
if base.Flag.LowerM >= 1 {
354-
base.WarnfAt(n.Pos(), "zero-copy string->[]byte conversion")
355-
}
356-
n.SetOp(ir.OSTR2BYTESTMP)
348+
if n, ok := n.(*ir.ConvExpr); ok && n.Op() == ir.OSTR2BYTES && !loc.hasAttr(attrMutates) {
349+
if base.Flag.LowerM >= 1 {
350+
base.WarnfAt(n.Pos(), "zero-copy string->[]byte conversion")
357351
}
352+
n.SetOp(ir.OSTR2BYTESTMP)
358353
}
359354
}
360355
}

src/cmd/compile/internal/ssagen/ssa.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2659,6 +2659,14 @@ func (s *state) exprCheckPtr(n ir.Node, checkPtrOK bool) *ssa.Value {
26592659
n := n.(*ir.ConvExpr)
26602660
str := s.expr(n.X)
26612661
ptr := s.newValue1(ssa.OpStringPtr, s.f.Config.Types.BytePtr, str)
2662+
if !n.NonNil() {
2663+
// We need to ensure []byte("") evaluates to []byte{}, and not []byte(nil).
2664+
//
2665+
// TODO(mdempsky): Investigate using "len != 0" instead of "ptr != nil".
2666+
cond := s.newValue2(ssa.OpNeqPtr, types.Types[types.TBOOL], ptr, s.constNil(ptr.Type))
2667+
zerobase := s.newValue1A(ssa.OpAddr, ptr.Type, ir.Syms.Zerobase, s.sb)
2668+
ptr = s.ternary(cond, ptr, zerobase)
2669+
}
26622670
len := s.newValue1(ssa.OpStringLen, types.Types[types.TINT], str)
26632671
return s.newValue3(ssa.OpSliceMake, n.Type(), ptr, len, len)
26642672
case ir.OCFUNC:

src/cmd/compile/internal/walk/order.go

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -815,8 +815,14 @@ func (o *orderState) stmt(n ir.Node) {
815815
// Mark []byte(str) range expression to reuse string backing storage.
816816
// It is safe because the storage cannot be mutated.
817817
n := n.(*ir.RangeStmt)
818-
if n.X.Op() == ir.OSTR2BYTES {
819-
n.X.(*ir.ConvExpr).SetOp(ir.OSTR2BYTESTMP)
818+
if x, ok := n.X.(*ir.ConvExpr); ok {
819+
switch x.Op() {
820+
case ir.OSTR2BYTES:
821+
x.SetOp(ir.OSTR2BYTESTMP)
822+
fallthrough
823+
case ir.OSTR2BYTESTMP:
824+
x.MarkNonNil() // "range []byte(nil)" is fine
825+
}
820826
}
821827

822828
t := o.markTemp()

src/cmd/compile/internal/walk/switch.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -736,6 +736,7 @@ func stringSearch(expr ir.Node, cc []exprClause, out *ir.Nodes) {
736736
// Convert expr to a []int8
737737
slice := ir.NewConvExpr(base.Pos, ir.OSTR2BYTESTMP, types.NewSlice(types.Types[types.TINT8]), expr)
738738
slice.SetTypecheck(1) // legacy typechecker doesn't handle this op
739+
slice.MarkNonNil()
739740
// Load the byte we're splitting on.
740741
load := ir.NewIndexExpr(base.Pos, slice, ir.NewInt(base.Pos, int64(bestIdx)))
741742
// Compare with the value we're splitting on.

test/escape2.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1729,7 +1729,7 @@ func intstring2() {
17291729

17301730
func stringtoslicebyte0() {
17311731
s := "foo"
1732-
x := []byte(s) // ERROR "\(\[\]byte\)\(s\) does not escape$"
1732+
x := []byte(s) // ERROR "\(\[\]byte\)\(s\) does not escape$" "zero-copy string->\[\]byte conversion"
17331733
_ = x
17341734
}
17351735

test/escape2n.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1729,7 +1729,7 @@ func intstring2() {
17291729

17301730
func stringtoslicebyte0() {
17311731
s := "foo"
1732-
x := []byte(s) // ERROR "\(\[\]byte\)\(s\) does not escape$"
1732+
x := []byte(s) // ERROR "\(\[\]byte\)\(s\) does not escape$" "zero-copy string->\[\]byte conversion"
17331733
_ = x
17341734
}
17351735

0 commit comments

Comments
 (0)