From dc960d0bfeb6ce6abd0cfa2c096476d98e9ec7cd Mon Sep 17 00:00:00 2001 From: Jake Bailey Date: Fri, 5 Sep 2025 20:04:58 -0700 Subject: [PATCH] cmd/compile, reflect: further allow inlining of TypeFor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previous CLs optimized direct use of abi.Type, but reflect.Type is indirected, so was not benefiting. For TypeFor, we can use toRType directly without a nil check because the types are statically known. Normally, I'd think SSA would remove the nil check, but due to some oddity (specifically, late fuse being required to remove the nil check, but opt doesn't run that late) means that the nil check persists and gets in the way. Manually writing the code in this instance seems to fix the problem. It also exposed another problem; depending on the ordering, writeType could get to a type symbol before SSA, thereby preventing Extra from being created on the symbol for later lookups that don't go through TypeLinksym directly. In writeType, for non-shape types, call TypeLinksym to ensure that the type is set up for later callers. That change itself passed toolstash -cmp. All up, this stack put through compilecmp shows a lot of improvement in various reflect-using packages, and reflect itself. It is too big to fit in the commit message but here's some info: compilecmp master -> HEAD master (d767064170): cmd/compile: mark abi.PtrType.Elem sym as used HEAD (846a94c568): cmd/compile, reflect: further allow inlining of TypeFor file before after Δ % addr2line 3735911 3735391 -520 -0.014% asm 6382235 6382091 -144 -0.002% buildid 3608568 3608360 -208 -0.006% cgo 5951816 5951480 -336 -0.006% compile 28362080 28339772 -22308 -0.079% cover 6668686 6661414 -7272 -0.109% dist 4311961 4311425 -536 -0.012% fix 3771706 3771474 -232 -0.006% link 8686073 8684993 -1080 -0.012% nm 3715923 3715459 -464 -0.012% objdump 6074366 6073774 -592 -0.010% pack 3025653 3025277 -376 -0.012% pprof 18269485 18261653 -7832 -0.043% test2json 3442726 3438390 -4336 -0.126% trace 16984831 16981767 -3064 -0.018% vet 10701931 10696355 -5576 -0.052% total 133693951 133639075 -54876 -0.041% runtime runtime.stkobjinit 240 -> 165 (-31.25%) runtime [cmd/compile] runtime.stkobjinit 240 -> 165 (-31.25%) reflect reflect.Value.Seq2.func3 309 -> 245 (-20.71%) reflect.Value.Seq2.func1.1 281 -> 198 (-29.54%) reflect.Value.Seq.func1.1 242 -> 165 (-31.82%) reflect.Value.Seq2.func2 360 -> 285 (-20.83%) reflect.Value.Seq.func4 281 -> 239 (-14.95%) reflect.Value.Seq2.func4 399 -> 284 (-28.82%) reflect.Value.Seq.func2 271 -> 230 (-15.13%) reflect.TypeFor[go.shape.uint64] 33 -> 18 (-45.45%) reflect.Value.Seq.func3 219 -> 178 (-18.72%) reflect [cmd/compile] reflect.Value.Seq2.func2 360 -> 285 (-20.83%) reflect.Value.Seq.func4 281 -> 239 (-14.95%) reflect.Value.Seq.func2 271 -> 230 (-15.13%) reflect.Value.Seq.func1.1 242 -> 165 (-31.82%) reflect.Value.Seq2.func1.1 281 -> 198 (-29.54%) reflect.Value.Seq2.func3 309 -> 245 (-20.71%) reflect.Value.Seq.func3 219 -> 178 (-18.72%) reflect.TypeFor[go.shape.uint64] 33 -> 18 (-45.45%) reflect.Value.Seq2.func4 399 -> 284 (-28.82%) fmt fmt.(*pp).fmtBytes 1723 -> 1691 (-1.86%) database/sql/driver reflect.TypeFor[go.shape.interface 33 -> 18 (-45.45%) database/sql/driver.init 72 -> 57 (-20.83%) Change-Id: I9eb750cf0b7ebf532589f939431feb0a899e42ff Reviewed-on: https://go-review.googlesource.com/c/go/+/701301 Reviewed-by: Mark Freeman Reviewed-by: Keith Randall Auto-Submit: Michael Pratt Reviewed-by: Keith Randall LUCI-TryBot-Result: Go LUCI --- .../compile/internal/reflectdata/reflect.go | 4 ++++ src/reflect/type.go | 3 ++- test/codegen/reflect_type.go | 21 +++++++++++++++++++ 3 files changed, 27 insertions(+), 1 deletion(-) create mode 100644 test/codegen/reflect_type.go diff --git a/src/cmd/compile/internal/reflectdata/reflect.go b/src/cmd/compile/internal/reflectdata/reflect.go index 2849d4ee40f..15173c54dcc 100644 --- a/src/cmd/compile/internal/reflectdata/reflect.go +++ b/src/cmd/compile/internal/reflectdata/reflect.go @@ -717,6 +717,10 @@ func writeType(t *types.Type) *obj.LSym { } s.SetSiggen(true) + if !tbase.HasShape() { + TypeLinksym(t) // ensure lsym.Extra is set + } + if !NeedEmit(tbase) { if i := typecheck.BaseTypeIndex(t); i >= 0 { lsym.Pkg = tbase.Sym().Pkg.Prefix diff --git a/src/reflect/type.go b/src/reflect/type.go index cec8662c01a..fc6edb1e106 100644 --- a/src/reflect/type.go +++ b/src/reflect/type.go @@ -1314,7 +1314,8 @@ func TypeOf(i any) Type { // TypeFor returns the [Type] that represents the type argument T. func TypeFor[T any]() Type { - return toType(abi.TypeFor[T]()) + // toRType is safe to use here; type is never nil as T is statically known. + return toRType(abi.TypeFor[T]()) } // rtypeOf directly extracts the *rtype of the provided value. diff --git a/test/codegen/reflect_type.go b/test/codegen/reflect_type.go new file mode 100644 index 00000000000..b92a9567f14 --- /dev/null +++ b/test/codegen/reflect_type.go @@ -0,0 +1,21 @@ +// asmcheck + +// 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 codegen + +import "reflect" + +func intPtrTypeSize() uintptr { + // amd64:"MOVL\t[$]8,",-"CALL" + // arm64:"MOVD\t[$]8,",-"CALL" + return reflect.TypeFor[*int]().Size() +} + +func intPtrTypeKind() reflect.Kind { + // amd64:"MOVL\t[$]22,",-"CALL" + // arm64:"MOVD\t[$]22,",-"CALL" + return reflect.TypeFor[*int]().Kind() +}