cmd/compile: move flagalloc op splitting to rewrite rules

Flagalloc has the unenviable task of splitting
flag-generating ops that have been merged with loads
when the flags need to "spilled" (i.e. regenerated).
Since there weren't very many of them, there was a hard-coded list
of ops and bespoke code written to split them.

This change migrates load splitting into rewrite rules,
to make them easier to maintain.

Change-Id: I7750eafb888a802206c410f9c341b3133e7748b8
Reviewed-on: https://go-review.googlesource.com/c/go/+/166978
Run-TryBot: Josh Bleecher Snyder <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Keith Randall <[email protected]>

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

@@ -21,6 +21,7 @@ type Config struct {
  Types Types
  lowerBlock blockRewriter // lowering function
  lowerValue valueRewriter // lowering function
+ splitLoad valueRewriter // function for splitting merged load ops; only used on some architectures
  registers []Register // machine registers
  gpRegMask regMask // general purpose integer register mask
  fpRegMask regMask // floating point register mask
@@ -201,6 +202,7 @@ func NewConfig(arch string, types Types, ctxt *obj.Link, optimize bool) *Config
  c.RegSize = 8
  c.lowerBlock = rewriteBlockAMD64
  c.lowerValue = rewriteValueAMD64
+ c.splitLoad = rewriteValueAMD64splitload
  c.registers = registersAMD64[:]
  c.gpRegMask = gpRegMaskAMD64
  c.fpRegMask = fpRegMaskAMD64
@@ -212,6 +214,7 @@ func NewConfig(arch string, types Types, ctxt *obj.Link, optimize bool) *Config
  c.RegSize = 8
  c.lowerBlock = rewriteBlockAMD64
  c.lowerValue = rewriteValueAMD64
+ c.splitLoad = rewriteValueAMD64splitload
  c.registers = registersAMD64[:]
  c.gpRegMask = gpRegMaskAMD64
  c.fpRegMask = fpRegMaskAMD64
@@ -224,6 +227,7 @@ func NewConfig(arch string, types Types, ctxt *obj.Link, optimize bool) *Config
  c.RegSize = 4
  c.lowerBlock = rewriteBlock386
  c.lowerValue = rewriteValue386
+ c.splitLoad = rewriteValue386splitload
  c.registers = registers386[:]
  c.gpRegMask = gpRegMask386
  c.fpRegMask = fpRegMask386

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

@@ -4,30 +4,6 @@
 
 package ssa
 
-// When breaking up a combined load-compare to separated load and compare operations,
-// opLoad specifies the load operation, and opCmp specifies the compare operation.
-type typeCmdLoadMap struct {
- opLoad Op
- opCmp Op
-}
-
-var opCmpLoadMap = map[Op]typeCmdLoadMap{
- OpAMD64CMPQload: {OpAMD64MOVQload, OpAMD64CMPQ},
- OpAMD64CMPLload: {OpAMD64MOVLload, OpAMD64CMPL},
- OpAMD64CMPWload: {OpAMD64MOVWload, OpAMD64CMPW},
- OpAMD64CMPBload: {OpAMD64MOVBload, OpAMD64CMPB},
- Op386CMPLload: {Op386MOVLload, Op386CMPL},
- Op386CMPWload: {Op386MOVWload, Op386CMPW},
- Op386CMPBload: {Op386MOVBload, Op386CMPB},
- OpAMD64CMPQconstload: {OpAMD64MOVQload, OpAMD64CMPQconst},
- OpAMD64CMPLconstload: {OpAMD64MOVLload, OpAMD64CMPLconst},
- OpAMD64CMPWconstload: {OpAMD64MOVWload, OpAMD64CMPWconst},
- OpAMD64CMPBconstload: {OpAMD64MOVBload, OpAMD64CMPBconst},
- Op386CMPLconstload: {Op386MOVLload, Op386CMPLconst},
- Op386CMPWconstload: {Op386MOVWload, Op386CMPWconst},
- Op386CMPBconstload: {Op386MOVBload, Op386CMPBconst},
-}
-
 // flagalloc allocates the flag register among all the flag-generating
 // instructions. Flag values are recomputed if they need to be
 // spilled/restored.
@@ -142,67 +118,10 @@ func flagalloc(f *Func) {
 
  // If v will be spilled, and v uses memory, then we must split it
  // into a load + a flag generator.
- // TODO: figure out how to do this without arch-dependent code.
  if spill[v.ID] && v.MemoryArg() != nil {
- switch v.Op {
- case OpAMD64CMPQload:
- load := b.NewValue2IA(v.Pos, opCmpLoadMap[v.Op].opLoad, f.Config.Types.UInt64, v.AuxInt, v.Aux, v.Args[0], v.Args[2])
- v.Op = opCmpLoadMap[v.Op].opCmp
- v.AuxInt = 0
- v.Aux = nil
- v.SetArgs2(load, v.Args[1])
- case OpAMD64CMPLload, Op386CMPLload:
- load := b.NewValue2IA(v.Pos, opCmpLoadMap[v.Op].opLoad, f.Config.Types.UInt32, v.AuxInt, v.Aux, v.Args[0], v.Args[2])
- v.Op = opCmpLoadMap[v.Op].opCmp
- v.AuxInt = 0
- v.Aux = nil
- v.SetArgs2(load, v.Args[1])
- case OpAMD64CMPWload, Op386CMPWload:
- load := b.NewValue2IA(v.Pos, opCmpLoadMap[v.Op].opLoad, f.Config.Types.UInt16, v.AuxInt, v.Aux, v.Args[0], v.Args[2])
- v.Op = opCmpLoadMap[v.Op].opCmp
- v.AuxInt = 0
- v.Aux = nil
- v.SetArgs2(load, v.Args[1])
- case OpAMD64CMPBload, Op386CMPBload:
- load := b.NewValue2IA(v.Pos, opCmpLoadMap[v.Op].opLoad, f.Config.Types.UInt8, v.AuxInt, v.Aux, v.Args[0], v.Args[2])
- v.Op = opCmpLoadMap[v.Op].opCmp
- v.AuxInt = 0
- v.Aux = nil
- v.SetArgs2(load, v.Args[1])
-
- case OpAMD64CMPQconstload:
- vo := v.AuxValAndOff()
- load := b.NewValue2IA(v.Pos, opCmpLoadMap[v.Op].opLoad, f.Config.Types.UInt64, vo.Off(), v.Aux, v.Args[0], v.Args[1])
- v.Op = opCmpLoadMap[v.Op].opCmp
- v.AuxInt = vo.Val()
- v.Aux = nil
- v.SetArgs1(load)
- case OpAMD64CMPLconstload, Op386CMPLconstload:
- vo := v.AuxValAndOff()
- load := b.NewValue2IA(v.Pos, opCmpLoadMap[v.Op].opLoad, f.Config.Types.UInt32, vo.Off(), v.Aux, v.Args[0], v.Args[1])
- v.Op = opCmpLoadMap[v.Op].opCmp
- v.AuxInt = vo.Val()
- v.Aux = nil
- v.SetArgs1(load)
- case OpAMD64CMPWconstload, Op386CMPWconstload:
- vo := v.AuxValAndOff()
- load := b.NewValue2IA(v.Pos, opCmpLoadMap[v.Op].opLoad, f.Config.Types.UInt16, vo.Off(), v.Aux, v.Args[0], v.Args[1])
- v.Op = opCmpLoadMap[v.Op].opCmp
- v.AuxInt = vo.Val()
- v.Aux = nil
- v.SetArgs1(load)
- case OpAMD64CMPBconstload, Op386CMPBconstload:
- vo := v.AuxValAndOff()
- load := b.NewValue2IA(v.Pos, opCmpLoadMap[v.Op].opLoad, f.Config.Types.UInt8, vo.Off(), v.Aux, v.Args[0], v.Args[1])
- v.Op = opCmpLoadMap[v.Op].opCmp
- v.AuxInt = vo.Val()
- v.Aux = nil
- v.SetArgs1(load)
-
- default:
+ if !f.Config.splitLoad(v) {
  f.Fatalf("can't split flag generator: %s", v.LongString())
  }
-
  }
 
  // Make sure any flag arg of v is in the flags register.

src/cmd/compile/internal/ssa/gen/386splitload.rules

@@ -0,0 +1,9 @@
+// Copyright 2019 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.
+
+// See the top of AMD64splitload.rules for discussion of these rules.
+
+(CMP(L|W|B)load {sym} [off] ptr x mem) -> (CMP(L|W|B) (MOV(L|W|B)load {sym} [off] ptr mem) x)
+
+(CMP(L|W|B)constload {sym} [vo] ptr mem) -> (CMP(L|W|B)const (MOV(L|W|B)load {sym} [offOnly(vo)] ptr mem) [valOnly(vo)])

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

@@ -0,0 +1,16 @@
+// Copyright 2019 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.
+
+// This file contains rules used by flagalloc to split
+// a flag-generating merged load op into separate load and op.
+// Unlike with the other rules files, not all of these
+// rules will be applied to all values.
+// Rather, flagalloc will request for rules to be applied
+// to a particular problematic value.
+// These are often the exact inverse of rules in AMD64.rules,
+// only with the conditions removed.
+
+(CMP(Q|L|W|B)load {sym} [off] ptr x mem) -> (CMP(Q|L|W|B) (MOV(Q|L|W|B)load {sym} [off] ptr mem) x)
+
+(CMP(Q|L|W|B)constload {sym} [vo] ptr mem) -> (CMP(Q|L|W|B)const (MOV(Q|L|W|B)load {sym} [offOnly(vo)] ptr mem) [valOnly(vo)])

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

@@ -398,6 +398,7 @@ func (a arch) Name() string {
 func genLower() {
  for _, a := range archs {
  genRules(a)
+ genSplitLoadRules(a)
  }
 }
 

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

@@ -80,11 +80,19 @@ func (r Rule) parse() (match, cond, result string) {
  return match, cond, result
 }
 
-func genRules(arch arch) {
+func genRules(arch arch) { genRulesSuffix(arch, "") }
+func genSplitLoadRules(arch arch) { genRulesSuffix(arch, "splitload") }
+
+func genRulesSuffix(arch arch, suff string) {
  // Open input file.
- text, err := os.Open(arch.name + ".rules")
+ text, err := os.Open(arch.name + suff + ".rules")
  if err != nil {
- log.Fatalf("can't read rule file: %v", err)
+ if suff == "" {
+ // All architectures must have a plain rules file.
+ log.Fatalf("can't read rule file: %v", err)
+ }
+ // Some architectures have bonus rules files that others don't share. That's fine.
+ return
  }
 
  // oprules contains a list of rules for each block and opcode
@@ -122,7 +130,7 @@ func genRules(arch arch) {
  continue
  }
 
- loc := fmt.Sprintf("%s.rules:%d", arch.name, ruleLineno)
+ loc := fmt.Sprintf("%s%s.rules:%d", arch.name, suff, ruleLineno)
  for _, rule2 := range expandOr(rule) {
  for _, rule3 := range commute(rule2, arch) {
  r := Rule{rule: rule3, loc: loc}
@@ -156,7 +164,7 @@ func genRules(arch arch) {
 
  // Start output buffer, write header.
  w := new(bytes.Buffer)
- fmt.Fprintf(w, "// Code generated from gen/%s.rules; DO NOT EDIT.\n", arch.name)
+ fmt.Fprintf(w, "// Code generated from gen/%s%s.rules; DO NOT EDIT.\n", arch.name, suff)
  fmt.Fprintln(w, "// generated with: cd gen; go run *.go")
  fmt.Fprintln(w)
  fmt.Fprintln(w, "package ssa")
@@ -174,7 +182,7 @@ func genRules(arch arch) {
 
  const chunkSize = 10
  // Main rewrite routine is a switch on v.Op.
- fmt.Fprintf(w, "func rewriteValue%s(v *Value) bool {\n", arch.name)
+ fmt.Fprintf(w, "func rewriteValue%s%s(v *Value) bool {\n", arch.name, suff)
  fmt.Fprintf(w, "switch v.Op {\n")
  for _, op := range ops {
  fmt.Fprintf(w, "case %s:\n", op)
@@ -183,7 +191,7 @@ func genRules(arch arch) {
  if chunk > 0 {
  fmt.Fprint(w, " || ")
  }
- fmt.Fprintf(w, "rewriteValue%s_%s_%d(v)", arch.name, op, chunk)
+ fmt.Fprintf(w, "rewriteValue%s%s_%s_%d(v)", arch.name, suff, op, chunk)
  }
  fmt.Fprintln(w)
  }
@@ -243,7 +251,7 @@ func genRules(arch arch) {
  hasconfig := strings.Contains(body, "config.") || strings.Contains(body, "config)")
  hasfe := strings.Contains(body, "fe.")
  hastyps := strings.Contains(body, "typ.")
- fmt.Fprintf(w, "func rewriteValue%s_%s_%d(v *Value) bool {\n", arch.name, op, chunk)
+ fmt.Fprintf(w, "func rewriteValue%s%s_%s_%d(v *Value) bool {\n", arch.name, suff, op, chunk)
  if hasb || hasconfig || hasfe || hastyps {
  fmt.Fprintln(w, "b := v.Block")
  }
@@ -263,7 +271,7 @@ func genRules(arch arch) {
 
  // Generate block rewrite function. There are only a few block types
  // so we can make this one function with a switch.
- fmt.Fprintf(w, "func rewriteBlock%s(b *Block) bool {\n", arch.name)
+ fmt.Fprintf(w, "func rewriteBlock%s%s(b *Block) bool {\n", arch.name, suff)
  fmt.Fprintln(w, "config := b.Func.Config")
  fmt.Fprintln(w, "_ = config")
  fmt.Fprintln(w, "fe := b.Func.fe")
@@ -382,7 +390,7 @@ func genRules(arch arch) {
  }
 
  // Write to file
- err = ioutil.WriteFile("../rewrite"+arch.name+".go", src, 0666)
+ err = ioutil.WriteFile("../rewrite"+arch.name+suff+".go", src, 0666)
  if err != nil {
  log.Fatalf("can't write output: %v\n", err)
  }

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

@@ -154,6 +154,18 @@ func makeValAndOff(val, off int64) int64 {
  return ValAndOff(val<<32 + int64(uint32(off))).Int64()
 }
 
+// offOnly returns the offset half of ValAndOff vo.
+// It is intended for use in rewrite rules.
+func offOnly(vo int64) int64 {
+ return ValAndOff(vo).Off()
+}
+
+// valOnly returns the value half of ValAndOff vo.
+// It is intended for use in rewrite rules.
+func valOnly(vo int64) int64 {
+ return ValAndOff(vo).Val()
+}
+
 func (x ValAndOff) canAdd(off int64) bool {
  newoff := x.Off() + off
  return newoff == int64(int32(newoff))