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: b80a6b28fb7c86c66ea65282702b3aa032d6f5a5
GitHub-Pull-Request: golang/go#66793
Reviewed-on: https://go-review.googlesource.com/c/go/+/578376
Reviewed-by: Keith Randall <khr@golang.org>
Auto-Submit: Keith Randall <khr@golang.org>
Reviewed-by: Joedian Reid <joedian@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Keith Randall <khr@google.com>
diff --git a/src/cmd/compile/internal/ssa/deadstore.go b/src/cmd/compile/internal/ssa/deadstore.go
index cb34271..ce04cb3 100644
--- a/src/cmd/compile/internal/ssa/deadstore.go
+++ b/src/cmd/compile/internal/ssa/deadstore.go
@@ -21,12 +21,18 @@
defer f.retSparseSet(storeUse)
shadowed := f.newSparseMap(f.NumValues())
defer f.retSparseMap(shadowed)
+ // 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.
+ localAddrs := map[any]*Value{}
for _, b := range f.Blocks {
// Find all the stores in this block. Categorize their uses:
// loadUse contains stores which are used by a subsequent load.
// storeUse contains stores which are used by a subsequent store.
loadUse.clear()
storeUse.clear()
+ // TODO(deparker): use the 'clear' builtin once compiler bootstrap minimum version is raised to 1.21.
+ for k := range localAddrs {
+ delete(localAddrs, k)
+ }
stores = stores[:0]
for _, v := range b.Values {
if v.Op == OpPhi {
@@ -46,6 +52,13 @@
}
}
} else {
+ if v.Op == OpLocalAddr {
+ if _, ok := localAddrs[v.Aux]; !ok {
+ localAddrs[v.Aux] = v
+ } else {
+ continue
+ }
+ }
for _, a := range v.Args {
if a.Block == b && a.Type.IsMemory() {
loadUse.add(a.ID)
@@ -100,6 +113,11 @@
} else { // OpZero
sz = v.AuxInt
}
+ if ptr.Op == OpLocalAddr {
+ if la, ok := localAddrs[ptr.Aux]; ok {
+ ptr = la
+ }
+ }
sr := shadowRange(shadowed.get(ptr.ID))
if sr.contains(off, off+sz) {
// Modify the store/zero into a copy of the memory state,
@@ -146,6 +164,7 @@
func (sr shadowRange) lo() int64 {
return int64(sr & 0xffff)
}
+
func (sr shadowRange) hi() int64 {
return int64((sr >> 16) & 0xffff)
}
diff --git a/src/cmd/compile/internal/ssa/deadstore_test.go b/src/cmd/compile/internal/ssa/deadstore_test.go
index 33cb4b9..4ccd6b8 100644
--- a/src/cmd/compile/internal/ssa/deadstore_test.go
+++ b/src/cmd/compile/internal/ssa/deadstore_test.go
@@ -6,6 +6,7 @@
import (
"cmd/compile/internal/types"
+ "cmd/internal/src"
"testing"
)
@@ -44,6 +45,7 @@
t.Errorf("dead store (zero) not removed")
}
}
+
func TestDeadStorePhi(t *testing.T) {
// make sure we don't get into an infinite loop with phi values.
c := testConfig(t)
@@ -127,3 +129,46 @@
t.Errorf("store %s incorrectly removed", v)
}
}
+
+func TestDeadStoreSmallStructInit(t *testing.T) {
+ c := testConfig(t)
+ ptrType := c.config.Types.BytePtr
+ typ := types.NewStruct([]*types.Field{
+ types.NewField(src.NoXPos, &types.Sym{Name: "A"}, c.config.Types.Int),
+ types.NewField(src.NoXPos, &types.Sym{Name: "B"}, c.config.Types.Int),
+ })
+ name := c.Temp(typ)
+ fun := c.Fun("entry",
+ Bloc("entry",
+ Valu("start", OpInitMem, types.TypeMem, 0, nil),
+ Valu("sp", OpSP, c.config.Types.Uintptr, 0, nil),
+ Valu("zero", OpConst64, c.config.Types.Int, 0, nil),
+ Valu("v6", OpLocalAddr, ptrType, 0, name, "sp", "start"),
+ Valu("v3", OpOffPtr, ptrType, 8, nil, "v6"),
+ Valu("v22", OpOffPtr, ptrType, 0, nil, "v6"),
+ Valu("zerostore1", OpStore, types.TypeMem, 0, c.config.Types.Int, "v22", "zero", "start"),
+ Valu("zerostore2", OpStore, types.TypeMem, 0, c.config.Types.Int, "v3", "zero", "zerostore1"),
+ Valu("v8", OpLocalAddr, ptrType, 0, name, "sp", "zerostore2"),
+ Valu("v23", OpOffPtr, ptrType, 8, nil, "v8"),
+ Valu("v25", OpOffPtr, ptrType, 0, nil, "v8"),
+ Valu("zerostore3", OpStore, types.TypeMem, 0, c.config.Types.Int, "v25", "zero", "zerostore2"),
+ Valu("zerostore4", OpStore, types.TypeMem, 0, c.config.Types.Int, "v23", "zero", "zerostore3"),
+ Goto("exit")),
+ Bloc("exit",
+ Exit("zerostore4")))
+
+ fun.f.Name = "smallstructinit"
+ CheckFunc(fun.f)
+ cse(fun.f)
+ dse(fun.f)
+ CheckFunc(fun.f)
+
+ v1 := fun.values["zerostore1"]
+ if v1.Op != OpCopy {
+ t.Errorf("dead store not removed")
+ }
+ v2 := fun.values["zerostore2"]
+ if v2.Op != OpCopy {
+ t.Errorf("dead store not removed")
+ }
+}
