Skip to content

Commit a034809

Browse files
dr2chasedr2chase
committed
cmd/compile: provide more names for stack slots
Recurse into structs/arrays of one element when assigning names. Test incorporated into existing end-to-end debugger test, hand-verified that it fails without this CL. Fixes golang#19868 Revives CL 40010 Old-Change-Id: I0266e58af975fb64cfa17922be383b70f0a7ea96 Change-Id: I122ac2375931477769ec8d763607c1ec42d78a7f Reviewed-on: https://go-review.googlesource.com/71731 Run-TryBot: David Chase <drchase@google.com> Reviewed-by: Heschi Kreinick <heschi@google.com> Reviewed-by: Keith Randall <khr@golang.org>
1 parent 018642d commit a034809

File tree

9 files changed

+450
-502
lines changed

9 files changed

+450
-502
lines changed

‎src/cmd/compile/internal/ssa/debug_test.go‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ import (
2525
var update = flag.Bool("u", false, "update test reference files")
2626
var verbose = flag.Bool("v", false, "print debugger interactions (very verbose)")
2727
var dryrun = flag.Bool("n", false, "just print the command line and first debugging bits")
28-
var delve = flag.Bool("d", false, "use Delve (dlv) instead of gdb, use dlv reverence files")
28+
var useDelve = flag.Bool("d", false, "use Delve (dlv) instead of gdb, use dlv reverence files")
2929
var force = flag.Bool("f", false, "force run under not linux-amd64; also do not use tempdir")
3030

3131
var repeats = flag.Bool("r", false, "detect repeats in debug steps and don't ignore them")
@@ -89,7 +89,7 @@ func TestNexting(t *testing.T) {
8989
}
9090
testenv.MustHaveGoBuild(t)
9191

92-
if !*delve && !*force && !(runtime.GOOS == "linux" && runtime.GOARCH == "amd64") {
92+
if !*useDelve && !*force && !(runtime.GOOS == "linux" && runtime.GOARCH == "amd64") {
9393
// Running gdb on OSX/darwin is very flaky.
9494
// Sometimes it is called ggdb, depending on how it is installed.
9595
// It also probably requires an admin password typed into a dialog box.
@@ -99,7 +99,7 @@ func TestNexting(t *testing.T) {
9999
skipReasons += "not run unless linux-amd64 or -d or -f; "
100100
}
101101

102-
if *delve {
102+
if *useDelve {
103103
debugger = "dlv"
104104
_, err := exec.LookPath("dlv")
105105
if err != nil {
@@ -132,11 +132,11 @@ func TestNexting(t *testing.T) {
132132
// If this is test is run with a runtime compiled with -N -l, it is very likely to fail.
133133
// This occurs in the noopt builders (for example).
134134
if gogcflags := os.Getenv("GO_GCFLAGS"); *force || (!strings.Contains(gogcflags, "-N") && !strings.Contains(gogcflags, "-l")) {
135-
if *delve || *inlines {
136-
testNexting(t, "hist", "opt", "")
135+
if *useDelve || *inlines {
136+
testNexting(t, "hist", "opt", "-dwarflocationlists")
137137
} else {
138138
// For gdb, disable inlining so that a compiler test does not depend on library code.
139-
testNexting(t, "hist", "opt", "-l")
139+
testNexting(t, "hist", "opt", "-l -dwarflocationlists")
140140
}
141141
} else {
142142
t.Skip("skipping for unoptimized runtime")
@@ -176,7 +176,7 @@ func testNexting(t *testing.T, base, tag, gcflags string) {
176176

177177
nextlog := logbase + "-" + debugger + ".nexts"
178178
tmplog := tmpbase + "-" + debugger + ".nexts"
179-
if *delve {
179+
if *useDelve {
180180
h1 = dlvTest(tag, exe, 1000)
181181
} else {
182182
h1 = gdbTest(tag, exe, 1000)

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

Lines changed: 74 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -233,40 +233,14 @@ func decomposeUser(f *Func) {
233233
// We must do the opt pass before any deadcode elimination or we will
234234
// lose the name->value correspondence.
235235
i := 0
236-
var fnames []LocalSlot
237236
var newNames []LocalSlot
238237
for _, name := range f.Names {
239238
t := name.Type
240239
switch {
241240
case t.IsStruct():
242-
n := t.NumFields()
243-
fnames = fnames[:0]
244-
for i := 0; i < n; i++ {
245-
fnames = append(fnames, f.fe.SplitStruct(name, i))
246-
}
247-
for _, v := range f.NamedValues[name] {
248-
for i := 0; i < n; i++ {
249-
x := v.Block.NewValue1I(v.Pos, OpStructSelect, t.FieldType(i), int64(i), v)
250-
f.NamedValues[fnames[i]] = append(f.NamedValues[fnames[i]], x)
251-
}
252-
}
253-
delete(f.NamedValues, name)
254-
newNames = append(newNames, fnames...)
241+
newNames = decomposeUserStructInto(f, name, newNames)
255242
case t.IsArray():
256-
if t.NumElem() == 0 {
257-
// TODO(khr): Not sure what to do here. Probably nothing.
258-
// Names for empty arrays aren't important.
259-
break
260-
}
261-
if t.NumElem() != 1 {
262-
f.Fatalf("array not of size 1")
263-
}
264-
elemName := f.fe.SplitArray(name)
265-
for _, v := range f.NamedValues[name] {
266-
e := v.Block.NewValue1I(v.Pos, OpArraySelect, t.ElemType(), 0, v)
267-
f.NamedValues[elemName] = append(f.NamedValues[elemName], e)
268-
}
269-
243+
newNames = decomposeUserArrayInto(f, name, newNames)
270244
default:
271245
f.Names[i] = name
272246
i++
@@ -276,6 +250,78 @@ func decomposeUser(f *Func) {
276250
f.Names = append(f.Names, newNames...)
277251
}
278252

253+
// decomposeUserArrayInto creates names for the element(s) of arrays referenced
254+
// by name where possible, and appends those new names to slots, which is then
255+
// returned.
256+
func decomposeUserArrayInto(f *Func, name LocalSlot, slots []LocalSlot) []LocalSlot {
257+
t := name.Type
258+
if t.NumElem() == 0 {
259+
// TODO(khr): Not sure what to do here. Probably nothing.
260+
// Names for empty arrays aren't important.
261+
return slots
262+
}
263+
if t.NumElem() != 1 {
264+
// shouldn't get here due to CanSSA
265+
f.Fatalf("array not of size 1")
266+
}
267+
elemName := f.fe.SplitArray(name)
268+
for _, v := range f.NamedValues[name] {
269+
e := v.Block.NewValue1I(v.Pos, OpArraySelect, t.ElemType(), 0, v)
270+
f.NamedValues[elemName] = append(f.NamedValues[elemName], e)
271+
}
272+
// delete the name for the array as a whole
273+
delete(f.NamedValues, name)
274+
275+
if t.ElemType().IsArray() {
276+
return decomposeUserArrayInto(f, elemName, slots)
277+
} else if t.ElemType().IsStruct() {
278+
return decomposeUserStructInto(f, elemName, slots)
279+
}
280+
281+
return append(slots, elemName)
282+
}
283+
284+
// decomposeUserStructInto creates names for the fields(s) of structs referenced
285+
// by name where possible, and appends those new names to slots, which is then
286+
// returned.
287+
func decomposeUserStructInto(f *Func, name LocalSlot, slots []LocalSlot) []LocalSlot {
288+
fnames := []LocalSlot{} // slots for struct in name
289+
t := name.Type
290+
n := t.NumFields()
291+
292+
for i := 0; i < n; i++ {
293+
fs := f.fe.SplitStruct(name, i)
294+
fnames = append(fnames, fs)
295+
// arrays and structs will be decomposed further, so
296+
// there's no need to record a name
297+
if !fs.Type.IsArray() && !fs.Type.IsStruct() {
298+
slots = append(slots, fs)
299+
}
300+
}
301+
302+
// create named values for each struct field
303+
for _, v := range f.NamedValues[name] {
304+
for i := 0; i < len(fnames); i++ {
305+
x := v.Block.NewValue1I(v.Pos, OpStructSelect, t.FieldType(i), int64(i), v)
306+
f.NamedValues[fnames[i]] = append(f.NamedValues[fnames[i]], x)
307+
}
308+
}
309+
// remove the name of the struct as a whole
310+
delete(f.NamedValues, name)
311+
312+
// now that this f.NamedValues contains values for the struct
313+
// fields, recurse into nested structs
314+
for i := 0; i < n; i++ {
315+
if name.Type.FieldType(i).IsStruct() {
316+
slots = decomposeUserStructInto(f, fnames[i], slots)
317+
delete(f.NamedValues, fnames[i])
318+
} else if name.Type.FieldType(i).IsArray() {
319+
slots = decomposeUserArrayInto(f, fnames[i], slots)
320+
delete(f.NamedValues, fnames[i])
321+
}
322+
}
323+
return slots
324+
}
279325
func decomposeUserPhi(v *Value) {
280326
switch {
281327
case v.Type.IsStruct():

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -875,8 +875,6 @@
875875
(Store _ (ArrayMake0) mem) -> mem
876876
(Store dst (ArrayMake1 e) mem) -> (Store {e.Type} dst e mem)
877877

878-
(ArraySelect [0] (Load ptr mem)) -> (Load ptr mem)
879-
880878
// Putting [1]{*byte} and similar into direct interfaces.
881879
(IMake typ (ArrayMake1 val)) -> (IMake typ val)
882880
(ArraySelect [0] x:(IData _)) -> x

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

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

0 commit comments

Comments
 (0)