runtime,cmd/compile: miss an optimization case for "struct{byte}"

[zigo101]

What version of Go are you using (go version)?

$ go version
go version go1.17.3 linux/amd64

Does this issue reproduce with the latest release?

Yes

What did you do?

package interfaces

import "testing"

var r interface{}

type T8 struct{n byte}
func Benchmark_ByteField(b *testing.B) {
	for i := 0; i < b.N; i++ { r = T8{byte(i&255)} }
}

type T16 struct{n int16}
func Benchmark_Int16Field(b *testing.B) {
	for i := 0; i < b.N; i++ { r = T16{int16(i&255)} }
}

type T32 struct{n int32}
func Benchmark_Int32Field(b *testing.B) {
	for i := 0; i < b.N; i++ { r = T32{int32(i&255)} }
}

type T64 struct{n int64}
func Benchmark_Int64Field(b *testing.B) {
	for i := 0; i < b.N; i++ { r = T64{int64(i&255)} }
}

What did you expect to see?

Similar value boxing costs.

What did you see instead?

Boxing struct{n byte} values is more costly than others.

goos: linux
goarch: amd64
Benchmark_ByteField-4    	60815828	        19.89 ns/op
Benchmark_Int16Field-4   	341708020	         3.424 ns/op
Benchmark_Int32Field-4   	348755539	         3.403 ns/op
Benchmark_Int64Field-4   	348057927	         3.637 ns/op

[mknyszek]

CC @randall77 @dr2chase

[cuonglm]

It seems to me that we need to extend dataWord to handle Type.SoleComponent() when fromType.Size() == 1:

Benchmark_ByteField-8    	1000000000	         0.6414 ns/op	       0 B/op	       0 allocs/op
Benchmark_Uint8Field-8   	1000000000	         0.6495 ns/op	       0 B/op	       0 allocs/op
Benchmark_Int16Field-8   	787386027	         1.590 ns/op	       0 B/op	       0 allocs/op
Benchmark_Int32Field-8   	663530320	         1.573 ns/op	       0 B/op	       0 allocs/op
Benchmark_Int64Field-8   	808163055	         1.574 ns/op	       0 B/op	       0 allocs/op

[randall77]

Sounds good. Can dataWord just do fromType := n.Type().SoleComponent() at its first line? Then all the cases handle wrapping with struct{}.

[randall77]

Or,

   fromType := n.Type()
   if s := fromType.SoleComponent(); s != nil { fromType = s }

[randall77]

Then dataWordFuncName doesn't need to call SoleComponent, as its arg is already normalized like that.

[cuonglm]

@randall77 dataWordFuncName is also called in order.go, so it still needs to call SoleComponent, unless we duplicate your code above in order.go, too.

[randall77]

A little bit unfortunate. We can lift to both callsites and then add an assertion in dataWordFuncName that its arg is already SoleComponented.

[gopherbot]

Change https://golang.org/cl/367755 mentions this issue: cmd/compile: handle sole component for 1-byte type interface conversion

[cuonglm]

A little bit unfortunate. We can lift to both callsites and then add an assertion in dataWordFuncName that its arg is already SoleComponented.

I sent https://go-review.googlesource.com/c/go/+/367755 for quick prototyping. I'm a bit nervous with adding the assertion in dataWordFuncName:

if sc := fromType.SoleComponent(); sc != nil && sc == fromType {
    base.Fatalf("type is not sole component-ed")
}

looks a bit ugly to me.