[dev.ssa] cmd/compile: Allow structs to be SSAd

Break small structs up into their components so they
can be registerized.

Change StructSelect to use field indexes instead of
field offsets, as field offsets aren't unique in the
presence of zero-sized fields.

Change-Id: I2f1dc89f7fa58e1cf58aa1a32b238959d53f62e4
Reviewed-on: https://go-review.googlesource.com/18570
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: David Chase <drchase@google.com>

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

@@ -1802,7 +1802,11 @@ func (s *state) expr(n *Node) *ssa.Value {
 		return s.newValue2(ssa.OpLoad, n.Type, p, s.mem())
 
 	case ODOT:
-		// TODO: fix when we can SSA struct types.
+		t := n.Left.Type
+		if canSSAType(t) {
+			v := s.expr(n.Left)
+			return s.newValue1I(ssa.OpStructSelect, n.Type, fieldIdx(n), v)
+		}
 		p := s.addr(n, false)
 		return s.newValue2(ssa.OpLoad, n.Type, p, s.mem())
 
@@ -1876,6 +1880,7 @@ func (s *state) expr(n *Node) *ssa.Value {
 		// such types being spilled.
 		// So here we ensure that we are selecting the underlying pointer
 		// when we build an eface.
+		// TODO: get rid of this now that structs can be SSA'd?
 		for !data.Type.IsPtr() {
 			switch {
 			case data.Type.IsArray():
@@ -1887,7 +1892,7 @@ func (s *state) expr(n *Node) *ssa.Value {
 						// eface type could also be struct{p *byte; q [0]int}
 						continue
 					}
-					data = s.newValue1I(ssa.OpStructSelect, f, data.Type.FieldOff(i), data)
+					data = s.newValue1I(ssa.OpStructSelect, f, i, data)
 					break
 				}
 			default:
@@ -2093,7 +2098,42 @@ func (s *state) assign(left *Node, right *ssa.Value, wb bool, line int32) {
 		}
 		right = s.zeroVal(t)
 	}
-	if left.Op == ONAME && canSSA(left) {
+	if canSSA(left) {
+		if left.Op == ODOT {
+			// We're assigning to a field of an ssa-able value.
+			// We need to build a new structure with the new value for the
+			// field we're assigning and the old values for the other fields.
+			// For instance:
+			//   type T struct {a, b, c int}
+			//   var T x
+			//   x.b = 5
+			// For the x.b = 5 assignment we want to generate x = T{x.a, 5, x.c}
+
+			// Grab information about the structure type.
+			t := left.Left.Type
+			nf := t.NumFields()
+			idx := fieldIdx(left)
+
+			// Grab old value of structure.
+			old := s.expr(left.Left)
+
+			// Make new structure.
+			new := s.newValue0(ssa.StructMakeOp(t.NumFields()), t)
+
+			// Add fields as args.
+			for i := int64(0); i < nf; i++ {
+				if i == idx {
+					new.AddArg(right)
+				} else {
+					new.AddArg(s.newValue1I(ssa.OpStructSelect, t.FieldType(i), i, old))
+				}
+			}
+
+			// Recursively assign the new value we've made to the base of the dot op.
+			s.assign(left.Left, new, false, line)
+			// TODO: do we need to update named values here?
+			return
+		}
 		// Update variable assignment.
 		s.vars[left] = right
 		s.addNamedValue(left, right)
@@ -2157,6 +2197,13 @@ func (s *state) zeroVal(t *Type) *ssa.Value {
 		return s.entryNewValue0(ssa.OpConstInterface, t)
 	case t.IsSlice():
 		return s.entryNewValue0(ssa.OpConstSlice, t)
+	case t.IsStruct():
+		n := t.NumFields()
+		v := s.entryNewValue0(ssa.StructMakeOp(t.NumFields()), t)
+		for i := int64(0); i < n; i++ {
+			v.AddArg(s.zeroVal(t.FieldType(i).(*Type)))
+		}
+		return v
 	}
 	s.Unimplementedf("zero for type %v not implemented", t)
 	return nil
@@ -2440,8 +2487,11 @@ func (s *state) addr(n *Node, bounded bool) *ssa.Value {
 }
 
 // canSSA reports whether n is SSA-able.
-// n must be an ONAME.
+// n must be an ONAME (or an ODOT sequence with an ONAME base).
 func canSSA(n *Node) bool {
+	for n.Op == ODOT {
+		n = n.Left
+	}
 	if n.Op != ONAME {
 		return false
 	}
@@ -2485,10 +2535,7 @@ func canSSAType(t *Type) bool {
 		// introduced by the compiler for variadic functions.
 		return false
 	case TSTRUCT:
-		if countfield(t) > 4 {
-			// 4 is an arbitrary constant.  Same reasoning
-			// as above, lots of small fields would waste
-			// register space needed by other values.
+		if countfield(t) > ssa.MaxStruct {
 			return false
 		}
 		for t1 := t.Type; t1 != nil; t1 = t1.Down {
@@ -2496,8 +2543,7 @@ func canSSAType(t *Type) bool {
 				return false
 			}
 		}
-		return false // until it is implemented
-		//return true
+		return true
 	default:
 		return true
 	}
@@ -4558,6 +4604,34 @@ func autoVar(v *ssa.Value) (*Node, int64) {
 	return loc.N.(*Node), loc.Off
 }
 
+// fieldIdx finds the index of the field referred to by the ODOT node n.
+func fieldIdx(n *Node) int64 {
+	t := n.Left.Type
+	f := n.Right
+	if t.Etype != TSTRUCT {
+		panic("ODOT's LHS is not a struct")
+	}
+
+	var i int64
+	for t1 := t.Type; t1 != nil; t1 = t1.Down {
+		if t1.Etype != TFIELD {
+			panic("non-TFIELD in TSTRUCT")
+		}
+		if t1.Sym != f.Sym {
+			i++
+			continue
+		}
+		if t1.Width != n.Xoffset {
+			panic("field offset doesn't match")
+		}
+		return i
+	}
+	panic(fmt.Sprintf("can't find field in expr %s\n", n))
+
+	// TODO: keep the result of this fucntion somewhere in the ODOT Node
+	// so we don't have to recompute it each time we need it.
+}
+
 // ssaExport exports a bunch of compiler services for the ssa backend.
 type ssaExport struct {
 	log           bool

src/cmd/compile/internal/ssa/TODO

@@ -9,6 +9,8 @@ Correctness
 - Debugging info (check & fix as much as we can)
 - Fix write barriers so cgo tests work (misc/cgo/errors/ptr.go)
 - Re-enable TestStackBarrierProfiling (src/runtime/pprof/pprof_test.go)
+- @ directive in rewrites might read overwritten data.  Save @loc
+  in variable before modifying v.
 
 Optimizations (better compiled code)
 ------------------------------------
@@ -28,7 +30,7 @@ Optimizations (better compiled code)
 - Use better write barrier calls
 - If there are a lot of MOVQ $0, ..., then load
   0 into a register and use the register as the source instead.
-- Allow structs (and arrays of length 1?) to be SSAable.
+- Allow arrays of length 1 (or longer, with all constant indexes?) to be SSAable.
 - Figure out how to make PARAMOUT variables ssa-able.
   They need to get spilled automatically at end-of-function somehow.
 - If strings are being passed around without being interpreted (ptr

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

@@ -13,23 +13,9 @@ func decompose(f *Func) {
 			if v.Op != OpPhi {
 				continue
 			}
-			switch {
-			case v.Type.IsComplex():
-				decomposeComplexPhi(v)
-			case v.Type.IsString():
-				decomposeStringPhi(v)
-			case v.Type.IsSlice():
-				decomposeSlicePhi(v)
-			case v.Type.IsInterface():
-				decomposeInterfacePhi(v)
-				//case v.Type.IsStruct():
-				//	decomposeStructPhi(v)
-			case v.Type.Size() > f.Config.IntSize:
-				f.Unimplementedf("undecomposed type %s", v.Type)
-			}
+			decomposePhi(v)
 		}
 	}
-	// TODO: decompose 64-bit ops on 32-bit archs?
 
 	// Split up named values into their components.
 	// NOTE: the component values we are making are dead at this point.
@@ -92,14 +78,39 @@ func decompose(f *Func) {
 				f.NamedValues[typeName] = append(f.NamedValues[typeName], typ)
 				f.NamedValues[dataName] = append(f.NamedValues[dataName], data)
 			}
-			//case t.IsStruct():
-			// TODO
+		case t.IsStruct():
+			n := t.NumFields()
+			for _, v := range f.NamedValues[name] {
+				for i := int64(0); i < n; i++ {
+					fname := LocalSlot{name.N, t.FieldType(i), name.Off + t.FieldOff(i)} // TODO: use actual field name?
+					x := v.Block.NewValue1I(v.Line, OpStructSelect, t.FieldType(i), i, v)
+					f.NamedValues[fname] = append(f.NamedValues[fname], x)
+				}
+			}
 		case t.Size() > f.Config.IntSize:
-			f.Unimplementedf("undecomposed type %s", t)
+			f.Unimplementedf("undecomposed named type %s", t)
 		}
 	}
 }
 
+func decomposePhi(v *Value) {
+	// TODO: decompose 64-bit ops on 32-bit archs?
+	switch {
+	case v.Type.IsComplex():
+		decomposeComplexPhi(v)
+	case v.Type.IsString():
+		decomposeStringPhi(v)
+	case v.Type.IsSlice():
+		decomposeSlicePhi(v)
+	case v.Type.IsInterface():
+		decomposeInterfacePhi(v)
+	case v.Type.IsStruct():
+		decomposeStructPhi(v)
+	case v.Type.Size() > v.Block.Func.Config.IntSize:
+		v.Unimplementedf("undecomposed type %s", v.Type)
+	}
+}
+
 func decomposeStringPhi(v *Value) {
 	fe := v.Block.Func.Config.fe
 	ptrType := fe.TypeBytePtr()
@@ -184,5 +195,47 @@ func decomposeInterfacePhi(v *Value) {
 	v.AddArg(data)
 }
 func decomposeStructPhi(v *Value) {
-	// TODO
+	t := v.Type
+	n := t.NumFields()
+	var fields [MaxStruct]*Value
+	for i := int64(0); i < n; i++ {
+		fields[i] = v.Block.NewValue0(v.Line, OpPhi, t.FieldType(i))
+	}
+	for _, a := range v.Args {
+		for i := int64(0); i < n; i++ {
+			fields[i].AddArg(a.Block.NewValue1I(v.Line, OpStructSelect, t.FieldType(i), i, a))
+		}
+	}
+	v.Op = StructMakeOp(n)
+	v.AuxInt = 0
+	v.Aux = nil
+	v.resetArgs()
+	v.AddArgs(fields[:n]...)
+
+	// Recursively decompose phis for each field.
+	for _, f := range fields[:n] {
+		decomposePhi(f)
+	}
+}
+
+// MaxStruct is the maximum number of fields a struct
+// can have and still be SSAable.
+const MaxStruct = 4
+
+// StructMakeOp returns the opcode to construct a struct with the
+// given number of fields.
+func StructMakeOp(nf int64) Op {
+	switch nf {
+	case 0:
+		return OpStructMake0
+	case 1:
+		return OpStructMake1
+	case 2:
+		return OpStructMake2
+	case 3:
+		return OpStructMake3
+	case 4:
+		return OpStructMake4
+	}
+	panic("too many fields in an SSAable struct")
 }

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

@@ -150,7 +150,70 @@
 (ArrayIndex (Load ptr mem) idx) && b == v.Args[0].Block -> (Load (PtrIndex <v.Type.PtrTo()> ptr idx) mem)
 (PtrIndex <t> ptr idx) && config.PtrSize == 4 -> (AddPtr ptr (Mul32 <config.fe.TypeInt()> idx (Const32 <config.fe.TypeInt()> [t.Elem().Size()])))
 (PtrIndex <t> ptr idx) && config.PtrSize == 8 -> (AddPtr ptr (Mul64 <config.fe.TypeInt()> idx (Const64 <config.fe.TypeInt()> [t.Elem().Size()])))
-(StructSelect [idx] (Load ptr mem)) -> @v.Args[0].Block (Load <v.Type> (OffPtr <v.Type.PtrTo()> [idx] ptr) mem)
+
+// struct operations
+(StructSelect (StructMake1 x)) -> x
+(StructSelect [0] (StructMake2 x _)) -> x
+(StructSelect [1] (StructMake2 _ x)) -> x
+(StructSelect [0] (StructMake3 x _ _)) -> x
+(StructSelect [1] (StructMake3 _ x _)) -> x
+(StructSelect [2] (StructMake3 _ _ x)) -> x
+(StructSelect [0] (StructMake4 x _ _ _)) -> x
+(StructSelect [1] (StructMake4 _ x _ _)) -> x
+(StructSelect [2] (StructMake4 _ _ x _)) -> x
+(StructSelect [3] (StructMake4 _ _ _ x)) -> x
+
+(Load <t> _ _) && t.IsStruct() && t.NumFields() == 0 && config.fe.CanSSA(t) ->
+  (StructMake0)
+(Load <t> ptr mem) && t.IsStruct() && t.NumFields() == 1 && config.fe.CanSSA(t) ->
+  (StructMake1
+    (Load <t.FieldType(0)> ptr mem))
+(Load <t> ptr mem) && t.IsStruct() && t.NumFields() == 2 && config.fe.CanSSA(t) ->
+  (StructMake2
+    (Load <t.FieldType(0)> ptr mem)
+    (Load <t.FieldType(1)> (OffPtr <t.FieldType(1).PtrTo()> [t.FieldOff(1)] ptr) mem))
+(Load <t> ptr mem) && t.IsStruct() && t.NumFields() == 3 && config.fe.CanSSA(t) ->
+  (StructMake3
+    (Load <t.FieldType(0)> ptr mem)
+    (Load <t.FieldType(1)> (OffPtr <t.FieldType(1).PtrTo()> [t.FieldOff(1)] ptr) mem)
+    (Load <t.FieldType(2)> (OffPtr <t.FieldType(2).PtrTo()> [t.FieldOff(2)] ptr) mem))
+(Load <t> ptr mem) && t.IsStruct() && t.NumFields() == 4 && config.fe.CanSSA(t) ->
+  (StructMake4
+    (Load <t.FieldType(0)> ptr mem)
+    (Load <t.FieldType(1)> (OffPtr <t.FieldType(1).PtrTo()> [t.FieldOff(1)] ptr) mem)
+    (Load <t.FieldType(2)> (OffPtr <t.FieldType(2).PtrTo()> [t.FieldOff(2)] ptr) mem)
+    (Load <t.FieldType(3)> (OffPtr <t.FieldType(3).PtrTo()> [t.FieldOff(3)] ptr) mem))
+
+(StructSelect [i] (Load <t> ptr mem)) && !config.fe.CanSSA(t) ->
+  @v.Args[0].Block (Load <v.Type> (OffPtr <v.Type.PtrTo()> [t.FieldOff(i)] ptr) mem)
+
+(Store _ (StructMake0) mem) -> mem
+(Store dst (StructMake1 <t> f0) mem) ->
+  (Store [t.FieldType(0).Size()] dst f0 mem)
+(Store dst (StructMake2 <t> f0 f1) mem) ->
+  (Store [t.FieldType(1).Size()]
+    (OffPtr <t.FieldType(1).PtrTo()> [t.FieldOff(1)] dst)
+    f1
+    (Store [t.FieldType(0).Size()] dst f0 mem))
+(Store dst (StructMake3 <t> f0 f1 f2) mem) ->
+  (Store [t.FieldType(2).Size()]
+    (OffPtr <t.FieldType(2).PtrTo()> [t.FieldOff(2)] dst)
+    f2
+    (Store [t.FieldType(1).Size()]
+      (OffPtr <t.FieldType(1).PtrTo()> [t.FieldOff(1)] dst)
+      f1
+      (Store [t.FieldType(0).Size()] dst f0 mem)))
+(Store dst (StructMake4 <t> f0 f1 f2 f3) mem) ->
+  (Store [t.FieldType(3).Size()]
+    (OffPtr <t.FieldType(3).PtrTo()> [t.FieldOff(3)] dst)
+    f3
+    (Store [t.FieldType(2).Size()]
+      (OffPtr <t.FieldType(2).PtrTo()> [t.FieldOff(2)] dst)
+      f2
+      (Store [t.FieldType(1).Size()]
+        (OffPtr <t.FieldType(1).PtrTo()> [t.FieldOff(1)] dst)
+        f1
+        (Store [t.FieldType(0).Size()] dst f0 mem))))
 
 // complex ops
 (ComplexReal (ComplexMake real _  )) -> real
@@ -303,3 +366,24 @@
   (ComplexMake
     (Arg <config.fe.TypeFloat32()> {n} [off])
     (Arg <config.fe.TypeFloat32()> {n} [off+4]))
+
+(Arg <t>) && t.IsStruct() && t.NumFields() == 0 && config.fe.CanSSA(t) ->
+  (StructMake0)
+(Arg <t> {n} [off]) && t.IsStruct() && t.NumFields() == 1 && config.fe.CanSSA(t) ->
+  (StructMake1
+    (Arg <t.FieldType(0)> {n} [off+t.FieldOff(0)]))
+(Arg <t> {n} [off]) && t.IsStruct() && t.NumFields() == 2 && config.fe.CanSSA(t) ->
+  (StructMake2
+    (Arg <t.FieldType(0)> {n} [off+t.FieldOff(0)])
+    (Arg <t.FieldType(1)> {n} [off+t.FieldOff(1)]))
+(Arg <t> {n} [off]) && t.IsStruct() && t.NumFields() == 3 && config.fe.CanSSA(t) ->
+  (StructMake3
+    (Arg <t.FieldType(0)> {n} [off+t.FieldOff(0)])
+    (Arg <t.FieldType(1)> {n} [off+t.FieldOff(1)])
+    (Arg <t.FieldType(2)> {n} [off+t.FieldOff(2)]))
+(Arg <t> {n} [off]) && t.IsStruct() && t.NumFields() == 4 && config.fe.CanSSA(t) ->
+  (StructMake4
+    (Arg <t.FieldType(0)> {n} [off+t.FieldOff(0)])
+    (Arg <t.FieldType(1)> {n} [off+t.FieldOff(1)])
+    (Arg <t.FieldType(2)> {n} [off+t.FieldOff(2)])
+    (Arg <t.FieldType(3)> {n} [off+t.FieldOff(3)]))

src/cmd/compile/internal/ssa/gen/genericOps.go

@@ -338,7 +338,6 @@ var genericOps = []opData{
 	{name: "ArrayIndex"}, // arg0=array, arg1=index.  Returns a[i]
 	{name: "PtrIndex"},   // arg0=ptr, arg1=index. Computes ptr+sizeof(*v.type)*index, where index is extended to ptrwidth type
 	{name: "OffPtr"},     // arg0 + auxint (arg0 and result are pointers)
-	{name: "StructSelect"}, // arg0=struct, auxint=field offset.  Returns field at that offset (size=size of result type)
 
 	// Slices
 	{name: "SliceMake"},                // arg0=ptr, arg1=len, arg2=cap
@@ -361,6 +360,14 @@ var genericOps = []opData{
 	{name: "ITab", typ: "BytePtr"}, // arg0=interface, returns itable field
 	{name: "IData"},                // arg0=interface, returns data field
 
+	// Structs
+	{name: "StructMake0"},  // Returns struct with 0 fields.
+	{name: "StructMake1"},  // arg0=field0.  Returns struct.
+	{name: "StructMake2"},  // arg0,arg1=field0,field1.  Returns struct.
+	{name: "StructMake3"},  // arg0..2=field0..2.  Returns struct.
+	{name: "StructMake4"},  // arg0..3=field0..3.  Returns struct.
+	{name: "StructSelect"}, // arg0=struct, auxint=field index.  Returns the auxint'th field.
+
 	// Spill&restore ops for the register allocator.  These are
 	// semantically identical to OpCopy; they do not take/return
 	// stores like regular memory ops do.  We can get away without memory

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

@@ -533,7 +533,6 @@ const (
 	OpArrayIndex
 	OpPtrIndex
 	OpOffPtr
-	OpStructSelect
 	OpSliceMake
 	OpSlicePtr
 	OpSliceLen
@@ -547,6 +546,12 @@ const (
 	OpIMake
 	OpITab
 	OpIData
+	OpStructMake0
+	OpStructMake1
+	OpStructMake2
+	OpStructMake3
+	OpStructMake4
+	OpStructSelect
 	OpStoreReg
 	OpLoadReg
 	OpFwdRef
@@ -4236,10 +4241,6 @@ var opcodeTable = [...]opInfo{
 		name:    "OffPtr",
 		generic: true,
 	},
-	{
-		name:    "StructSelect",
-		generic: true,
-	},
 	{
 		name:    "SliceMake",
 		generic: true,
@@ -4292,6 +4293,30 @@ var opcodeTable = [...]opInfo{
 		name:    "IData",
 		generic: true,
 	},
+	{
+		name:    "StructMake0",
+		generic: true,
+	},
+	{
+		name:    "StructMake1",
+		generic: true,
+	},
+	{
+		name:    "StructMake2",
+		generic: true,
+	},
+	{
+		name:    "StructMake3",
+		generic: true,
+	},
+	{
+		name:    "StructMake4",
+		generic: true,
+	},
+	{
+		name:    "StructSelect",
+		generic: true,
+	},
 	{
 		name:    "StoreReg",
 		generic: true,