Skip to content

Commit 03ed439

Browse files
committed
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. Merged revert-of-revert for 4 CLs. original revert 681937 695016 693415 694996 693615 695015 694195 694995 Fixes #74092 Update #74888 Update #74908 Update #74935 (updated issues are bugs in the last attempt at this) Change-Id: I32246d49b8bac3bb080972dc06ab432a5480d560 Reviewed-on: https://go-review.googlesource.com/c/go/+/714421 Auto-Submit: Keith Randall <[email protected]> Reviewed-by: David Chase <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]> Reviewed-by: Keith Randall <[email protected]>
1 parent 1bb1f2b commit 03ed439

File tree

13 files changed

+191
-49
lines changed

13 files changed

+191
-49
lines changed

src/cmd/compile/internal/ssa/_gen/dec.rules

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -97,8 +97,10 @@
9797
// Helpers for expand calls
9898
// Some of these are copied from generic.rules
9999

100-
(IMake _typ (StructMake val)) => (IMake _typ val)
101-
(StructSelect [0] (IData x)) => (IData x)
100+
(IMake _typ (StructMake ___)) => imakeOfStructMake(v)
101+
(StructSelect (IData x)) && v.Type.Size() > 0 => (IData x)
102+
(StructSelect (IData x)) && v.Type.Size() == 0 && v.Type.IsStruct() => (StructMake)
103+
(StructSelect (IData x)) && v.Type.Size() == 0 && v.Type.IsArray() => (ArrayMake0)
102104

103105
(StructSelect [i] x:(StructMake ___)) => x.Args[i]
104106

@@ -109,7 +111,7 @@
109111
// More annoying case: (ArraySelect[0] (StructSelect[0] isAPtr))
110112
// There, result of the StructSelect is an Array (not a pointer) and
111113
// the pre-rewrite input to the ArraySelect is a struct, not a pointer.
112-
(StructSelect [0] x) && x.Type.IsPtrShaped() => x
114+
(StructSelect x) && x.Type.IsPtrShaped() => x
113115
(ArraySelect [0] x) && x.Type.IsPtrShaped() => x
114116

115117
// These, too. Bits is bits.
@@ -119,6 +121,7 @@
119121

120122
(Store _ (StructMake ___) _) => rewriteStructStore(v)
121123

124+
(IMake _typ (ArrayMake1 val)) => (IMake _typ val)
122125
(ArraySelect (ArrayMake1 x)) => x
123126
(ArraySelect [0] (IData x)) => (IData x)
124127

src/cmd/compile/internal/ssa/_gen/generic.rules

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -944,8 +944,10 @@
944944
@x.Block (Load <v.Type> (OffPtr <v.Type.PtrTo()> [t.FieldOff(int(i))] ptr) mem)
945945

946946
// Putting struct{*byte} and similar into direct interfaces.
947-
(IMake _typ (StructMake val)) => (IMake _typ val)
948-
(StructSelect [0] (IData x)) => (IData x)
947+
(IMake _typ (StructMake ___)) => imakeOfStructMake(v)
948+
(StructSelect (IData x)) && v.Type.Size() > 0 => (IData x)
949+
(StructSelect (IData x)) && v.Type.Size() == 0 && v.Type.IsStruct() => (StructMake)
950+
(StructSelect (IData x)) && v.Type.Size() == 0 && v.Type.IsArray() => (ArrayMake0)
949951

950952
// un-SSAable values use mem->mem copies
951953
(Store {t} dst (Load src mem) mem) && !CanSSA(t) =>

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

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -423,7 +423,14 @@ func (x *expandState) decomposeAsNecessary(pos src.XPos, b *Block, a, m0 *Value,
423423
if a.Op == OpIMake {
424424
data := a.Args[1]
425425
for data.Op == OpStructMake || data.Op == OpArrayMake1 {
426-
data = data.Args[0]
426+
// A struct make might have a few zero-sized fields.
427+
// Use the pointer-y one we know is there.
428+
for _, a := range data.Args {
429+
if a.Type.Size() > 0 {
430+
data = a
431+
break
432+
}
433+
}
427434
}
428435
return x.decomposeAsNecessary(pos, b, data, mem, rc.next(data.Type))
429436
}

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2772,3 +2772,17 @@ func panicBoundsCCToAux(p PanicBoundsCC) Aux {
27722772
func isDictArgSym(sym Sym) bool {
27732773
return sym.(*ir.Name).Sym().Name == typecheck.LocalDictName
27742774
}
2775+
2776+
// When v is (IMake typ (StructMake ...)), convert to
2777+
// (IMake typ arg) where arg is the pointer-y argument to
2778+
// the StructMake (there must be exactly one).
2779+
func imakeOfStructMake(v *Value) *Value {
2780+
var arg *Value
2781+
for _, a := range v.Args[1].Args {
2782+
if a.Type.Size() > 0 {
2783+
arg = a
2784+
break
2785+
}
2786+
}
2787+
return v.Block.NewValue2(v.Pos, OpIMake, v.Type, v.Args[0], arg)
2788+
}

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

Lines changed: 44 additions & 8 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

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

Lines changed: 36 additions & 9 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/cmd/compile/internal/types/type.go

Lines changed: 1 addition & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1822,26 +1822,7 @@ func IsReflexive(t *Type) bool {
18221822
// Can this type be stored directly in an interface word?
18231823
// Yes, if the representation is a single pointer.
18241824
func IsDirectIface(t *Type) bool {
1825-
switch t.Kind() {
1826-
case TPTR:
1827-
// Pointers to notinheap types must be stored indirectly. See issue 42076.
1828-
return !t.Elem().NotInHeap()
1829-
case TCHAN,
1830-
TMAP,
1831-
TFUNC,
1832-
TUNSAFEPTR:
1833-
return true
1834-
1835-
case TARRAY:
1836-
// Array of 1 direct iface type can be direct.
1837-
return t.NumElem() == 1 && IsDirectIface(t.Elem())
1838-
1839-
case TSTRUCT:
1840-
// Struct with 1 field of direct iface type can be direct.
1841-
return t.NumFields() == 1 && IsDirectIface(t.Field(0).Type)
1842-
}
1843-
1844-
return false
1825+
return t.Size() == int64(PtrSize) && PtrDataSize(t) == int64(PtrSize)
18451826
}
18461827

18471828
// IsInterfaceMethod reports whether (field) m is

src/internal/abi/type.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -121,8 +121,8 @@ const (
121121
TFlagGCMaskOnDemand TFlag = 1 << 4
122122

123123
// TFlagDirectIface means that a value of this type is stored directly
124-
// in the data field of an interface, instead of indirectly. Normally
125-
// this means the type is pointer-ish.
124+
// in the data field of an interface, instead of indirectly.
125+
// This flag is just a cached computation of Size_ == PtrBytes == goarch.PtrSize.
126126
TFlagDirectIface TFlag = 1 << 5
127127

128128
// Leaving this breadcrumb behind for dlv. It should not be used, and no

src/reflect/type.go

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2528,8 +2528,7 @@ func StructOf(fields []StructField) Type {
25282528
}
25292529

25302530
switch {
2531-
case len(fs) == 1 && fs[0].Typ.IsDirectIface():
2532-
// structs of 1 direct iface type can be direct
2531+
case typ.Size_ == goarch.PtrSize && typ.PtrBytes == goarch.PtrSize:
25332532
typ.TFlag |= abi.TFlagDirectIface
25342533
default:
25352534
typ.TFlag &^= abi.TFlagDirectIface
@@ -2698,8 +2697,7 @@ func ArrayOf(length int, elem Type) Type {
26982697
}
26992698

27002699
switch {
2701-
case length == 1 && typ.IsDirectIface():
2702-
// array of 1 direct iface type can be direct
2700+
case array.Size_ == goarch.PtrSize && array.PtrBytes == goarch.PtrSize:
27032701
array.TFlag |= abi.TFlagDirectIface
27042702
default:
27052703
array.TFlag &^= abi.TFlagDirectIface

src/reflect/value.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1279,6 +1279,17 @@ func (v Value) Field(i int) Value {
12791279
fl |= flagStickyRO
12801280
}
12811281
}
1282+
if fl&flagIndir == 0 && typ.Size() == 0 {
1283+
// Special case for picking a field out of a direct struct.
1284+
// A direct struct must have a pointer field and possibly a
1285+
// bunch of zero-sized fields. We must return the zero-sized
1286+
// fields indirectly, as only ptr-shaped things can be direct.
1287+
// See issue 74935.
1288+
// We use nil instead of v.ptr as it doesn't matter and
1289+
// we can avoid pinning a possibly now-unused object.
1290+
return Value{typ, nil, fl | flagIndir}
1291+
}
1292+
12821293
// Either flagIndir is set and v.ptr points at struct,
12831294
// or flagIndir is not set and v.ptr is the actual struct data.
12841295
// In the former case, we want v.ptr + offset.

0 commit comments

Comments
 (0)