[release-branch.go1.7] cmd/compile: compare size in dead store elimination

This CL is a manual backpatch of CL 27320 into the 1.7.1 release branch.

The manual backpatch is required because OpZero changed from having a
size as its AuxInt to having a size+align as its AuxInt (that was to support
the ARM SSA backend).  Otherwise the CLs should be identical.

Please review carefully!

Change-Id: I569b759c06d1971c9c62dc5dd589abc7ef7c844a
Reviewed-on: https://go-review.googlesource.com/28670
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: David Chase <drchase@google.com>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

diff --git a/src/cmd/compile/internal/ssa/deadstore.go b/src/cmd/compile/internal/ssa/deadstore.go
index 5129c171bbf2506ce19befad99ab9a1bdc05ed3e..59ac1b9ad4d3761b07784b408ba6d08b3b4c80b2 100644 (file)
--- a/src/cmd/compile/internal/ssa/deadstore.go
+++ b/src/cmd/compile/internal/ssa/deadstore.go
@@ -14,8 +14,7 @@ func dse(f *Func) {
        defer f.retSparseSet(loadUse)
        storeUse := f.newSparseSet(f.NumValues())
        defer f.retSparseSet(storeUse)
-       shadowed := f.newSparseSet(f.NumValues())
-       defer f.retSparseSet(shadowed)
+       shadowed := newSparseMap(f.NumValues()) // TODO: cache
        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.
@@ -81,17 +80,18 @@ func dse(f *Func) {
                        shadowed.clear()
                }
                if v.Op == OpStore || v.Op == OpZero {
-                       if shadowed.contains(v.Args[0].ID) {
+                       sz := v.AuxInt
+                       if shadowedSize := int64(shadowed.get(v.Args[0].ID)); shadowedSize != -1 && shadowedSize >= sz {
                                // Modify store into a copy
                                if v.Op == OpStore {
                                        // store addr value mem
                                        v.SetArgs1(v.Args[2])
                                } else {
                                        // zero addr mem
-                                       sz := v.Args[0].Type.ElemType().Size()
-                                       if v.AuxInt != sz {
+                                       typesz := v.Args[0].Type.ElemType().Size()
+                                       if sz != typesz {
                                                f.Fatalf("mismatched zero/store sizes: %d and %d [%s]",
-                                                       v.AuxInt, sz, v.LongString())
+                                                       sz, typesz, v.LongString())
                                        }
                                        v.SetArgs1(v.Args[1])
                                }
@@ -99,7 +99,10 @@ func dse(f *Func) {
                                v.AuxInt = 0
                                v.Op = OpCopy
                        } else {
-                               shadowed.add(v.Args[0].ID)
+                               if sz > 0x7fffffff { // work around sparseMap's int32 value type
+                                       sz = 0x7fffffff
+                               }
+                               shadowed.set(v.Args[0].ID, int32(sz))
                        }
                }
                // walk to previous store

diff --git a/src/cmd/compile/internal/ssa/deadstore_test.go b/src/cmd/compile/internal/ssa/deadstore_test.go
index c38f1cdbaf7682605e3614c30d4bc4823ff2a937..beb6de063705a96a24cd3eb1d926d52b65a435e2 100644 (file)
--- a/src/cmd/compile/internal/ssa/deadstore_test.go
+++ b/src/cmd/compile/internal/ssa/deadstore_test.go
@@ -8,7 +8,7 @@ import "testing"
 
 func TestDeadStore(t *testing.T) {
        c := testConfig(t)
-       elemType := &TypeImpl{Size_: 8, Name: "testtype"}
+       elemType := &TypeImpl{Size_: 1, Name: "testtype"}
        ptrType := &TypeImpl{Size_: 8, Ptr: true, Name: "testptr", Elem_: elemType} // dummy for testing
        fun := Fun(c, "entry",
                Bloc("entry",
@@ -18,7 +18,7 @@ func TestDeadStore(t *testing.T) {
                        Valu("addr1", OpAddr, ptrType, 0, nil, "sb"),
                        Valu("addr2", OpAddr, ptrType, 0, nil, "sb"),
                        Valu("addr3", OpAddr, ptrType, 0, nil, "sb"),
-                       Valu("zero1", OpZero, TypeMem, 8, nil, "addr3", "start"),
+                       Valu("zero1", OpZero, TypeMem, 1, nil, "addr3", "start"),
                        Valu("store1", OpStore, TypeMem, 1, nil, "addr1", "v", "zero1"),
                        Valu("store2", OpStore, TypeMem, 1, nil, "addr2", "v", "store1"),
                        Valu("store3", OpStore, TypeMem, 1, nil, "addr1", "v", "store2"),
@@ -95,3 +95,32 @@ func TestDeadStoreTypes(t *testing.T) {
                t.Errorf("store %s incorrectly removed", v)
        }
 }
+
+func TestDeadStoreUnsafe(t *testing.T) {
+       // Make sure a narrow store can't shadow a wider one. The test above
+       // covers the case of two different types, but unsafe pointer casting
+       // can get to a point where the size is changed but type unchanged.
+       c := testConfig(t)
+       ptrType := &TypeImpl{Size_: 8, Ptr: true, Name: "testptr"} // dummy for testing
+       fun := Fun(c, "entry",
+               Bloc("entry",
+                       Valu("start", OpInitMem, TypeMem, 0, nil),
+                       Valu("sb", OpSB, TypeInvalid, 0, nil),
+                       Valu("v", OpConstBool, TypeBool, 1, nil),
+                       Valu("addr1", OpAddr, ptrType, 0, nil, "sb"),
+                       Valu("store1", OpStore, TypeMem, 8, nil, "addr1", "v", "start"),  // store 8 bytes
+                       Valu("store2", OpStore, TypeMem, 1, nil, "addr1", "v", "store1"), // store 1 byte
+                       Goto("exit")),
+               Bloc("exit",
+                       Exit("store2")))
+
+       CheckFunc(fun.f)
+       cse(fun.f)
+       dse(fun.f)
+       CheckFunc(fun.f)
+
+       v := fun.values["store1"]
+       if v.Op == OpCopy {
+               t.Errorf("store %s incorrectly removed", v)
+       }
+}