Commit ed737fd8 authored by Keith Randall's avatar Keith Randall

[dev.ssa] cmd/compile: fix @ rewrite rules

The @ directive used to read the target block after some value
structure had already changed.  I don't think it was ever really
a bug, but it's confusing.

It might fail like this:

(Foo x y) -> @v.Args[0].Block (Bar y (Baz ...))

v.Op = Bar
v.Args[0] = y
v.Args[1] = v.Args[0].Block.NewValue(Baz, ...)

That new value is allocated in the block of y, not the
block of x.

Anyway, read the destination block first so this
potential bug can't happen.

Change-Id: Ie41d2fc349b35cefaa319fa9327808bcb781b4e2
Reviewed-on: https://go-review.googlesource.com/19900
Run-TryBot: Todd Neal <todd@tneal.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: default avatarTodd Neal <todd@tneal.org>
parent e173ab14
...@@ -7,8 +7,6 @@ Coverage ...@@ -7,8 +7,6 @@ Coverage
Correctness Correctness
----------- -----------
- Debugging info (check & fix as much as we can) - Debugging info (check & fix as much as we can)
- @ directive in rewrites might read overwritten data. Save @loc
in variable before modifying v.
Optimizations (better compiled code) Optimizations (better compiled code)
------------------------------------ ------------------------------------
......
...@@ -259,7 +259,7 @@ func genRules(arch arch) { ...@@ -259,7 +259,7 @@ func genRules(arch arch) {
if t[1] == "nil" { if t[1] == "nil" {
fmt.Fprintf(w, "b.Control = nil\n") fmt.Fprintf(w, "b.Control = nil\n")
} else { } else {
fmt.Fprintf(w, "b.Control = %s\n", genResult0(w, arch, t[1], new(int), false, "b")) fmt.Fprintf(w, "b.Control = %s\n", genResult0(w, arch, t[1], new(int), false, false))
} }
if len(newsuccs) < len(succs) { if len(newsuccs) < len(succs) {
fmt.Fprintf(w, "b.Succs = b.Succs[:%d]\n", len(newsuccs)) fmt.Fprintf(w, "b.Succs = b.Succs[:%d]\n", len(newsuccs))
...@@ -415,16 +415,17 @@ func genMatch0(w io.Writer, arch arch, match, v string, m map[string]string, top ...@@ -415,16 +415,17 @@ func genMatch0(w io.Writer, arch arch, match, v string, m map[string]string, top
} }
func genResult(w io.Writer, arch arch, result string) { func genResult(w io.Writer, arch arch, result string) {
loc := "b" move := false
if result[0] == '@' { if result[0] == '@' {
// parse @block directive // parse @block directive
s := strings.SplitN(result[1:], " ", 2) s := strings.SplitN(result[1:], " ", 2)
loc = s[0] fmt.Fprintf(w, "b = %s\n", s[0])
result = s[1] result = s[1]
move = true
} }
genResult0(w, arch, result, new(int), true, loc) genResult0(w, arch, result, new(int), true, move)
} }
func genResult0(w io.Writer, arch arch, result string, alloc *int, top bool, loc string) string { func genResult0(w io.Writer, arch arch, result string, alloc *int, top, move bool) string {
if result[0] != '(' { if result[0] != '(' {
// variable // variable
if top { if top {
...@@ -469,7 +470,7 @@ func genResult0(w io.Writer, arch arch, result string, alloc *int, top bool, loc ...@@ -469,7 +470,7 @@ func genResult0(w io.Writer, arch arch, result string, alloc *int, top bool, loc
} }
} }
var v string var v string
if top && loc == "b" { if top && !move {
v = "v" v = "v"
fmt.Fprintf(w, "v.reset(%s)\n", opName(s[0], arch)) fmt.Fprintf(w, "v.reset(%s)\n", opName(s[0], arch))
if typeOverride { if typeOverride {
...@@ -481,8 +482,8 @@ func genResult0(w io.Writer, arch arch, result string, alloc *int, top bool, loc ...@@ -481,8 +482,8 @@ func genResult0(w io.Writer, arch arch, result string, alloc *int, top bool, loc
} }
v = fmt.Sprintf("v%d", *alloc) v = fmt.Sprintf("v%d", *alloc)
*alloc++ *alloc++
fmt.Fprintf(w, "%s := %s.NewValue0(v.Line, %s, %s)\n", v, loc, opName(s[0], arch), opType) fmt.Fprintf(w, "%s := b.NewValue0(v.Line, %s, %s)\n", v, opName(s[0], arch), opType)
if top { if move {
// Rewrite original into a copy // Rewrite original into a copy
fmt.Fprintf(w, "v.reset(OpCopy)\n") fmt.Fprintf(w, "v.reset(OpCopy)\n")
fmt.Fprintf(w, "v.AddArg(%s)\n", v) fmt.Fprintf(w, "v.AddArg(%s)\n", v)
...@@ -501,7 +502,7 @@ func genResult0(w io.Writer, arch arch, result string, alloc *int, top bool, loc ...@@ -501,7 +502,7 @@ func genResult0(w io.Writer, arch arch, result string, alloc *int, top bool, loc
fmt.Fprintf(w, "%s.Aux = %s\n", v, x) fmt.Fprintf(w, "%s.Aux = %s\n", v, x)
} else { } else {
// regular argument (sexpr or variable) // regular argument (sexpr or variable)
x := genResult0(w, arch, a, alloc, false, loc) x := genResult0(w, arch, a, alloc, false, move)
fmt.Fprintf(w, "%s.AddArg(%s)\n", v, x) fmt.Fprintf(w, "%s.AddArg(%s)\n", v, x)
} }
} }
......
...@@ -5339,7 +5339,8 @@ func rewriteValueAMD64_OpAMD64MOVBQSX(v *Value, config *Config) bool { ...@@ -5339,7 +5339,8 @@ func rewriteValueAMD64_OpAMD64MOVBQSX(v *Value, config *Config) bool {
sym := v.Args[0].Aux sym := v.Args[0].Aux
ptr := v.Args[0].Args[0] ptr := v.Args[0].Args[0]
mem := v.Args[0].Args[1] mem := v.Args[0].Args[1]
v0 := v.Args[0].Block.NewValue0(v.Line, OpAMD64MOVBQSXload, v.Type) b = v.Args[0].Block
v0 := b.NewValue0(v.Line, OpAMD64MOVBQSXload, v.Type)
v.reset(OpCopy) v.reset(OpCopy)
v.AddArg(v0) v.AddArg(v0)
v0.AuxInt = off v0.AuxInt = off
...@@ -5381,7 +5382,8 @@ func rewriteValueAMD64_OpAMD64MOVBQZX(v *Value, config *Config) bool { ...@@ -5381,7 +5382,8 @@ func rewriteValueAMD64_OpAMD64MOVBQZX(v *Value, config *Config) bool {
sym := v.Args[0].Aux sym := v.Args[0].Aux
ptr := v.Args[0].Args[0] ptr := v.Args[0].Args[0]
mem := v.Args[0].Args[1] mem := v.Args[0].Args[1]
v0 := v.Args[0].Block.NewValue0(v.Line, OpAMD64MOVBQZXload, v.Type) b = v.Args[0].Block
v0 := b.NewValue0(v.Line, OpAMD64MOVBQZXload, v.Type)
v.reset(OpCopy) v.reset(OpCopy)
v.AddArg(v0) v.AddArg(v0)
v0.AuxInt = off v0.AuxInt = off
...@@ -5920,7 +5922,8 @@ func rewriteValueAMD64_OpAMD64MOVLQSX(v *Value, config *Config) bool { ...@@ -5920,7 +5922,8 @@ func rewriteValueAMD64_OpAMD64MOVLQSX(v *Value, config *Config) bool {
sym := v.Args[0].Aux sym := v.Args[0].Aux
ptr := v.Args[0].Args[0] ptr := v.Args[0].Args[0]
mem := v.Args[0].Args[1] mem := v.Args[0].Args[1]
v0 := v.Args[0].Block.NewValue0(v.Line, OpAMD64MOVLQSXload, v.Type) b = v.Args[0].Block
v0 := b.NewValue0(v.Line, OpAMD64MOVLQSXload, v.Type)
v.reset(OpCopy) v.reset(OpCopy)
v.AddArg(v0) v.AddArg(v0)
v0.AuxInt = off v0.AuxInt = off
...@@ -5962,7 +5965,8 @@ func rewriteValueAMD64_OpAMD64MOVLQZX(v *Value, config *Config) bool { ...@@ -5962,7 +5965,8 @@ func rewriteValueAMD64_OpAMD64MOVLQZX(v *Value, config *Config) bool {
sym := v.Args[0].Aux sym := v.Args[0].Aux
ptr := v.Args[0].Args[0] ptr := v.Args[0].Args[0]
mem := v.Args[0].Args[1] mem := v.Args[0].Args[1]
v0 := v.Args[0].Block.NewValue0(v.Line, OpAMD64MOVLQZXload, v.Type) b = v.Args[0].Block
v0 := b.NewValue0(v.Line, OpAMD64MOVLQZXload, v.Type)
v.reset(OpCopy) v.reset(OpCopy)
v.AddArg(v0) v.AddArg(v0)
v0.AuxInt = off v0.AuxInt = off
...@@ -7419,7 +7423,8 @@ func rewriteValueAMD64_OpAMD64MOVWQSX(v *Value, config *Config) bool { ...@@ -7419,7 +7423,8 @@ func rewriteValueAMD64_OpAMD64MOVWQSX(v *Value, config *Config) bool {
sym := v.Args[0].Aux sym := v.Args[0].Aux
ptr := v.Args[0].Args[0] ptr := v.Args[0].Args[0]
mem := v.Args[0].Args[1] mem := v.Args[0].Args[1]
v0 := v.Args[0].Block.NewValue0(v.Line, OpAMD64MOVWQSXload, v.Type) b = v.Args[0].Block
v0 := b.NewValue0(v.Line, OpAMD64MOVWQSXload, v.Type)
v.reset(OpCopy) v.reset(OpCopy)
v.AddArg(v0) v.AddArg(v0)
v0.AuxInt = off v0.AuxInt = off
...@@ -7461,7 +7466,8 @@ func rewriteValueAMD64_OpAMD64MOVWQZX(v *Value, config *Config) bool { ...@@ -7461,7 +7466,8 @@ func rewriteValueAMD64_OpAMD64MOVWQZX(v *Value, config *Config) bool {
sym := v.Args[0].Aux sym := v.Args[0].Aux
ptr := v.Args[0].Args[0] ptr := v.Args[0].Args[0]
mem := v.Args[0].Args[1] mem := v.Args[0].Args[1]
v0 := v.Args[0].Block.NewValue0(v.Line, OpAMD64MOVWQZXload, v.Type) b = v.Args[0].Block
v0 := b.NewValue0(v.Line, OpAMD64MOVWQZXload, v.Type)
v.reset(OpCopy) v.reset(OpCopy)
v.AddArg(v0) v.AddArg(v0)
v0.AuxInt = off v0.AuxInt = off
......
...@@ -7102,10 +7102,13 @@ func rewriteValuegeneric_OpStructSelect(v *Value, config *Config) bool { ...@@ -7102,10 +7102,13 @@ func rewriteValuegeneric_OpStructSelect(v *Value, config *Config) bool {
if !(!config.fe.CanSSA(t)) { if !(!config.fe.CanSSA(t)) {
break break
} }
v0 := v.Args[0].Block.NewValue0(v.Line, OpLoad, v.Type) b = v.Args[0].Block
v0 := b.NewValue0(v.Line, OpLoad, v.Type)
v.reset(OpCopy) v.reset(OpCopy)
v.AddArg(v0) v.AddArg(v0)
v1 := v.Args[0].Block.NewValue0(v.Line, OpOffPtr, v.Type.PtrTo()) v1 := b.NewValue0(v.Line, OpOffPtr, v.Type.PtrTo())
v.reset(OpCopy)
v.AddArg(v1)
v1.AuxInt = t.FieldOff(i) v1.AuxInt = t.FieldOff(i)
v1.AddArg(ptr) v1.AddArg(ptr)
v0.AddArg(v1) v0.AddArg(v1)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment