Skip to content

Commit 5702600

Browse files
derekparkergopherbot
authored andcommitted
cmd/compile: teach dse about equivalent LocalAddrs
This patch teaches DSE that two LocalAddrs of the same variable are equal, even if they are from different memory states. This avoids dependance on a store into the same LocalAddr being added to loadUse even though the store is unnecessary and is in fact shadowed. Fixes #59021 Change-Id: I0ef128b783c4ad6fd2236fa5ff20345b4d31eddb GitHub-Last-Rev: b80a6b2 GitHub-Pull-Request: #66793 Reviewed-on: https://go-review.googlesource.com/c/go/+/578376 Reviewed-by: Keith Randall <[email protected]> Auto-Submit: Keith Randall <[email protected]> Reviewed-by: Joedian Reid <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]> Reviewed-by: Keith Randall <[email protected]>
1 parent a8ba163 commit 5702600

File tree

2 files changed

+64
-0
lines changed

2 files changed

+64
-0
lines changed

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

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,18 @@ func dse(f *Func) {
2121
defer f.retSparseSet(storeUse)
2222
shadowed := f.newSparseMap(f.NumValues())
2323
defer f.retSparseMap(shadowed)
24+
// localAddrs maps from a local variable (the Aux field of a LocalAddr value) to an instance of a LocalAddr value for that variable in the current block.
25+
localAddrs := map[any]*Value{}
2426
for _, b := range f.Blocks {
2527
// Find all the stores in this block. Categorize their uses:
2628
// loadUse contains stores which are used by a subsequent load.
2729
// storeUse contains stores which are used by a subsequent store.
2830
loadUse.clear()
2931
storeUse.clear()
32+
// TODO(deparker): use the 'clear' builtin once compiler bootstrap minimum version is raised to 1.21.
33+
for k := range localAddrs {
34+
delete(localAddrs, k)
35+
}
3036
stores = stores[:0]
3137
for _, v := range b.Values {
3238
if v.Op == OpPhi {
@@ -46,6 +52,13 @@ func dse(f *Func) {
4652
}
4753
}
4854
} else {
55+
if v.Op == OpLocalAddr {
56+
if _, ok := localAddrs[v.Aux]; !ok {
57+
localAddrs[v.Aux] = v
58+
} else {
59+
continue
60+
}
61+
}
4962
for _, a := range v.Args {
5063
if a.Block == b && a.Type.IsMemory() {
5164
loadUse.add(a.ID)
@@ -100,6 +113,11 @@ func dse(f *Func) {
100113
} else { // OpZero
101114
sz = v.AuxInt
102115
}
116+
if ptr.Op == OpLocalAddr {
117+
if la, ok := localAddrs[ptr.Aux]; ok {
118+
ptr = la
119+
}
120+
}
103121
sr := shadowRange(shadowed.get(ptr.ID))
104122
if sr.contains(off, off+sz) {
105123
// Modify the store/zero into a copy of the memory state,
@@ -146,6 +164,7 @@ type shadowRange int32
146164
func (sr shadowRange) lo() int64 {
147165
return int64(sr & 0xffff)
148166
}
167+
149168
func (sr shadowRange) hi() int64 {
150169
return int64((sr >> 16) & 0xffff)
151170
}

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

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ package ssa
66

77
import (
88
"cmd/compile/internal/types"
9+
"cmd/internal/src"
910
"testing"
1011
)
1112

@@ -44,6 +45,7 @@ func TestDeadStore(t *testing.T) {
4445
t.Errorf("dead store (zero) not removed")
4546
}
4647
}
48+
4749
func TestDeadStorePhi(t *testing.T) {
4850
// make sure we don't get into an infinite loop with phi values.
4951
c := testConfig(t)
@@ -127,3 +129,46 @@ func TestDeadStoreUnsafe(t *testing.T) {
127129
t.Errorf("store %s incorrectly removed", v)
128130
}
129131
}
132+
133+
func TestDeadStoreSmallStructInit(t *testing.T) {
134+
c := testConfig(t)
135+
ptrType := c.config.Types.BytePtr
136+
typ := types.NewStruct([]*types.Field{
137+
types.NewField(src.NoXPos, &types.Sym{Name: "A"}, c.config.Types.Int),
138+
types.NewField(src.NoXPos, &types.Sym{Name: "B"}, c.config.Types.Int),
139+
})
140+
name := c.Temp(typ)
141+
fun := c.Fun("entry",
142+
Bloc("entry",
143+
Valu("start", OpInitMem, types.TypeMem, 0, nil),
144+
Valu("sp", OpSP, c.config.Types.Uintptr, 0, nil),
145+
Valu("zero", OpConst64, c.config.Types.Int, 0, nil),
146+
Valu("v6", OpLocalAddr, ptrType, 0, name, "sp", "start"),
147+
Valu("v3", OpOffPtr, ptrType, 8, nil, "v6"),
148+
Valu("v22", OpOffPtr, ptrType, 0, nil, "v6"),
149+
Valu("zerostore1", OpStore, types.TypeMem, 0, c.config.Types.Int, "v22", "zero", "start"),
150+
Valu("zerostore2", OpStore, types.TypeMem, 0, c.config.Types.Int, "v3", "zero", "zerostore1"),
151+
Valu("v8", OpLocalAddr, ptrType, 0, name, "sp", "zerostore2"),
152+
Valu("v23", OpOffPtr, ptrType, 8, nil, "v8"),
153+
Valu("v25", OpOffPtr, ptrType, 0, nil, "v8"),
154+
Valu("zerostore3", OpStore, types.TypeMem, 0, c.config.Types.Int, "v25", "zero", "zerostore2"),
155+
Valu("zerostore4", OpStore, types.TypeMem, 0, c.config.Types.Int, "v23", "zero", "zerostore3"),
156+
Goto("exit")),
157+
Bloc("exit",
158+
Exit("zerostore4")))
159+
160+
fun.f.Name = "smallstructinit"
161+
CheckFunc(fun.f)
162+
cse(fun.f)
163+
dse(fun.f)
164+
CheckFunc(fun.f)
165+
166+
v1 := fun.values["zerostore1"]
167+
if v1.Op != OpCopy {
168+
t.Errorf("dead store not removed")
169+
}
170+
v2 := fun.values["zerostore2"]
171+
if v2.Op != OpCopy {
172+
t.Errorf("dead store not removed")
173+
}
174+
}

0 commit comments

Comments
 (0)