From: Keith Randall Date: Sun, 15 Jun 2025 03:10:50 +0000 (-0700) Subject: cmd/compile: allow multi-field structs to be stored directly in interfaces X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=cd55f86b8dcfc139ee5c17d32530ac9e758c8bc0;p=gostls13.git cmd/compile: allow multi-field structs to be stored directly in interfaces If the struct is a bunch of 0-sized fields and one pointer field. Fixes #74092 Change-Id: I87c5d162c8c9fdba812420d7f9d21de97295b62c Reviewed-on: https://go-review.googlesource.com/c/go/+/681937 LUCI-TryBot-Result: Go LUCI Reviewed-by: David Chase Reviewed-by: Keith Randall --- diff --git a/src/cmd/compile/internal/ssa/_gen/generic.rules b/src/cmd/compile/internal/ssa/_gen/generic.rules index 7ac36c3b3c..8f354e977f 100644 --- a/src/cmd/compile/internal/ssa/_gen/generic.rules +++ b/src/cmd/compile/internal/ssa/_gen/generic.rules @@ -921,8 +921,8 @@ @x.Block (Load (OffPtr [t.FieldOff(int(i))] ptr) mem) // Putting struct{*byte} and similar into direct interfaces. -(IMake _typ (StructMake val)) => (IMake _typ val) -(StructSelect [0] (IData x)) => (IData x) +(IMake _typ (StructMake val)) => imakeOfStructMake(v) +(StructSelect [_] (IData x)) => (IData x) // un-SSAable values use mem->mem copies (Store {t} dst (Load src mem) mem) && !CanSSA(t) => diff --git a/src/cmd/compile/internal/ssa/expand_calls.go b/src/cmd/compile/internal/ssa/expand_calls.go index 272b653ca3..30606bb805 100644 --- a/src/cmd/compile/internal/ssa/expand_calls.go +++ b/src/cmd/compile/internal/ssa/expand_calls.go @@ -423,7 +423,14 @@ func (x *expandState) decomposeAsNecessary(pos src.XPos, b *Block, a, m0 *Value, if a.Op == OpIMake { data := a.Args[1] for data.Op == OpStructMake || data.Op == OpArrayMake1 { - data = data.Args[0] + // A struct make might have a few zero-sized fields. + // Use the pointer-y one we know is there. + for _, a := range data.Args { + if a.Type.Size() > 0 { + data = a + break + } + } } return x.decomposeAsNecessary(pos, b, data, mem, rc.next(data.Type)) } diff --git a/src/cmd/compile/internal/ssa/rewrite.go b/src/cmd/compile/internal/ssa/rewrite.go index 200318ceb5..87f7b517c2 100644 --- a/src/cmd/compile/internal/ssa/rewrite.go +++ b/src/cmd/compile/internal/ssa/rewrite.go @@ -2689,3 +2689,17 @@ func panicBoundsCToAux(p PanicBoundsC) Aux { func panicBoundsCCToAux(p PanicBoundsCC) Aux { return p } + +// When v is (IMake typ (StructMake ...)), convert to +// (IMake typ arg) where arg is the pointer-y argument to +// the StructMake (there must be exactly one). +func imakeOfStructMake(v *Value) *Value { + var arg *Value + for _, a := range v.Args[1].Args { + if a.Type.Size() > 0 { + arg = a + break + } + } + return v.Block.NewValue2(v.Pos, OpIMake, v.Type, v.Args[0], arg) +} diff --git a/src/cmd/compile/internal/ssa/rewritegeneric.go b/src/cmd/compile/internal/ssa/rewritegeneric.go index 3af7f40e68..83be129a7b 100644 --- a/src/cmd/compile/internal/ssa/rewritegeneric.go +++ b/src/cmd/compile/internal/ssa/rewritegeneric.go @@ -11201,15 +11201,12 @@ func rewriteValuegeneric_OpIMake(v *Value) bool { v_1 := v.Args[1] v_0 := v.Args[0] // match: (IMake _typ (StructMake val)) - // result: (IMake _typ val) + // result: imakeOfStructMake(v) for { - _typ := v_0 if v_1.Op != OpStructMake || len(v_1.Args) != 1 { break } - val := v_1.Args[0] - v.reset(OpIMake) - v.AddArg2(_typ, val) + v.copyOf(imakeOfStructMake(v)) return true } // match: (IMake _typ (ArrayMake1 val)) @@ -32045,10 +32042,10 @@ func rewriteValuegeneric_OpStructSelect(v *Value) bool { v0.AddArg2(v1, mem) return true } - // match: (StructSelect [0] (IData x)) + // match: (StructSelect [_] (IData x)) // result: (IData x) for { - if auxIntToInt64(v.AuxInt) != 0 || v_0.Op != OpIData { + if v_0.Op != OpIData { break } x := v_0.Args[0] diff --git a/src/cmd/compile/internal/types/type.go b/src/cmd/compile/internal/types/type.go index 1c7f0a19e9..d73b5e8d7b 100644 --- a/src/cmd/compile/internal/types/type.go +++ b/src/cmd/compile/internal/types/type.go @@ -1829,26 +1829,7 @@ func IsReflexive(t *Type) bool { // Can this type be stored directly in an interface word? // Yes, if the representation is a single pointer. func IsDirectIface(t *Type) bool { - switch t.Kind() { - case TPTR: - // Pointers to notinheap types must be stored indirectly. See issue 42076. - return !t.Elem().NotInHeap() - case TCHAN, - TMAP, - TFUNC, - TUNSAFEPTR: - return true - - case TARRAY: - // Array of 1 direct iface type can be direct. - return t.NumElem() == 1 && IsDirectIface(t.Elem()) - - case TSTRUCT: - // Struct with 1 field of direct iface type can be direct. - return t.NumFields() == 1 && IsDirectIface(t.Field(0).Type) - } - - return false + return t.Size() == int64(PtrSize) && PtrDataSize(t) == int64(PtrSize) } // IsInterfaceMethod reports whether (field) m is diff --git a/src/internal/abi/type.go b/src/internal/abi/type.go index 1920a8a37f..9f783a34c1 100644 --- a/src/internal/abi/type.go +++ b/src/internal/abi/type.go @@ -121,8 +121,8 @@ const ( TFlagGCMaskOnDemand TFlag = 1 << 4 // TFlagDirectIface means that a value of this type is stored directly - // in the data field of an interface, instead of indirectly. Normally - // this means the type is pointer-ish. + // in the data field of an interface, instead of indirectly. + // This flag is just a cached computation of Size_ == PtrBytes == goarch.PtrSize. TFlagDirectIface TFlag = 1 << 5 // Leaving this breadcrumb behind for dlv. It should not be used, and no diff --git a/src/reflect/type.go b/src/reflect/type.go index cec8662c01..19a28abfcf 100644 --- a/src/reflect/type.go +++ b/src/reflect/type.go @@ -2524,8 +2524,7 @@ func StructOf(fields []StructField) Type { } switch { - case len(fs) == 1 && fs[0].Typ.IsDirectIface(): - // structs of 1 direct iface type can be direct + case typ.Size_ == goarch.PtrSize && typ.PtrBytes == goarch.PtrSize: typ.TFlag |= abi.TFlagDirectIface default: typ.TFlag &^= abi.TFlagDirectIface @@ -2694,8 +2693,7 @@ func ArrayOf(length int, elem Type) Type { } switch { - case length == 1 && typ.IsDirectIface(): - // array of 1 direct iface type can be direct + case array.Size_ == goarch.PtrSize && array.PtrBytes == goarch.PtrSize: array.TFlag |= abi.TFlagDirectIface default: array.TFlag &^= abi.TFlagDirectIface