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 cb3427103c501bd818e053350de9c4cd18f7e3e5..ce04cb3a24ff72c8f22e67f8e64e206ec5392bda 100644 (file)
--- a/src/cmd/compile/internal/ssa/deadstore.go
+++ b/src/cmd/compile/internal/ssa/deadstore.go
@@ -21,12 +21,18 @@ func dse(f *Func) {
        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 @@ func dse(f *Func) {
                                        }
                                }
                        } 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 @@ func dse(f *Func) {
                        } 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 @@ type shadowRange int32
 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 33cb4b97553c0513b785a7b5cb73507901b7c402..4ccd6b8e91bf12cbacc1e1eefeedec8df20e0138 100644 (file)
--- a/src/cmd/compile/internal/ssa/deadstore_test.go
+++ b/src/cmd/compile/internal/ssa/deadstore_test.go
@@ -6,6 +6,7 @@ package ssa
 
 import (
        "cmd/compile/internal/types"
+       "cmd/internal/src"
        "testing"
 )
 
@@ -44,6 +45,7 @@ func TestDeadStore(t *testing.T) {
                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 @@ func TestDeadStoreUnsafe(t *testing.T) {
                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")
+       }
+}