From: Keith Randall Date: Wed, 24 Feb 2016 18:29:27 +0000 (-0800) Subject: [dev.ssa] cmd/compile: fix @ rewrite rules X-Git-Tag: go1.7beta1~1623^2^2~22 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=ed737fd8cdc1a668027bb5f5dac8879afabcca3b;p=gostls13.git [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 TryBot-Result: Gobot Gobot Reviewed-by: Todd Neal --- diff --git a/src/cmd/compile/internal/ssa/TODO b/src/cmd/compile/internal/ssa/TODO index 91983476a2..69356d6226 100644 --- a/src/cmd/compile/internal/ssa/TODO +++ b/src/cmd/compile/internal/ssa/TODO @@ -7,8 +7,6 @@ Coverage Correctness ----------- - 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) ------------------------------------ diff --git a/src/cmd/compile/internal/ssa/gen/rulegen.go b/src/cmd/compile/internal/ssa/gen/rulegen.go index b9aa51d165..56bb82c85d 100644 --- a/src/cmd/compile/internal/ssa/gen/rulegen.go +++ b/src/cmd/compile/internal/ssa/gen/rulegen.go @@ -259,7 +259,7 @@ func genRules(arch arch) { if t[1] == "nil" { fmt.Fprintf(w, "b.Control = nil\n") } 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) { 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 } func genResult(w io.Writer, arch arch, result string) { - loc := "b" + move := false if result[0] == '@' { // parse @block directive s := strings.SplitN(result[1:], " ", 2) - loc = s[0] + fmt.Fprintf(w, "b = %s\n", s[0]) 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] != '(' { // variable if top { @@ -469,7 +470,7 @@ func genResult0(w io.Writer, arch arch, result string, alloc *int, top bool, loc } } var v string - if top && loc == "b" { + if top && !move { v = "v" fmt.Fprintf(w, "v.reset(%s)\n", opName(s[0], arch)) if typeOverride { @@ -481,8 +482,8 @@ func genResult0(w io.Writer, arch arch, result string, alloc *int, top bool, loc } v = fmt.Sprintf("v%d", *alloc) *alloc++ - fmt.Fprintf(w, "%s := %s.NewValue0(v.Line, %s, %s)\n", v, loc, opName(s[0], arch), opType) - if top { + fmt.Fprintf(w, "%s := b.NewValue0(v.Line, %s, %s)\n", v, opName(s[0], arch), opType) + if move { // Rewrite original into a copy fmt.Fprintf(w, "v.reset(OpCopy)\n") 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 fmt.Fprintf(w, "%s.Aux = %s\n", v, x) } else { // 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) } } diff --git a/src/cmd/compile/internal/ssa/rewriteAMD64.go b/src/cmd/compile/internal/ssa/rewriteAMD64.go index 601e9b8ce3..bf74331dd3 100644 --- a/src/cmd/compile/internal/ssa/rewriteAMD64.go +++ b/src/cmd/compile/internal/ssa/rewriteAMD64.go @@ -5339,7 +5339,8 @@ func rewriteValueAMD64_OpAMD64MOVBQSX(v *Value, config *Config) bool { sym := v.Args[0].Aux ptr := v.Args[0].Args[0] 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.AddArg(v0) v0.AuxInt = off @@ -5381,7 +5382,8 @@ func rewriteValueAMD64_OpAMD64MOVBQZX(v *Value, config *Config) bool { sym := v.Args[0].Aux ptr := v.Args[0].Args[0] 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.AddArg(v0) v0.AuxInt = off @@ -5920,7 +5922,8 @@ func rewriteValueAMD64_OpAMD64MOVLQSX(v *Value, config *Config) bool { sym := v.Args[0].Aux ptr := v.Args[0].Args[0] 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.AddArg(v0) v0.AuxInt = off @@ -5962,7 +5965,8 @@ func rewriteValueAMD64_OpAMD64MOVLQZX(v *Value, config *Config) bool { sym := v.Args[0].Aux ptr := v.Args[0].Args[0] 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.AddArg(v0) v0.AuxInt = off @@ -7419,7 +7423,8 @@ func rewriteValueAMD64_OpAMD64MOVWQSX(v *Value, config *Config) bool { sym := v.Args[0].Aux ptr := v.Args[0].Args[0] 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.AddArg(v0) v0.AuxInt = off @@ -7461,7 +7466,8 @@ func rewriteValueAMD64_OpAMD64MOVWQZX(v *Value, config *Config) bool { sym := v.Args[0].Aux ptr := v.Args[0].Args[0] 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.AddArg(v0) v0.AuxInt = off diff --git a/src/cmd/compile/internal/ssa/rewritegeneric.go b/src/cmd/compile/internal/ssa/rewritegeneric.go index 08ab2e14a6..4f29cf5348 100644 --- a/src/cmd/compile/internal/ssa/rewritegeneric.go +++ b/src/cmd/compile/internal/ssa/rewritegeneric.go @@ -7102,10 +7102,13 @@ func rewriteValuegeneric_OpStructSelect(v *Value, config *Config) bool { if !(!config.fe.CanSSA(t)) { 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.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.AddArg(ptr) v0.AddArg(v1)