diff options
author | Keith Randall <khr@golang.org> | 2016-09-07 10:57:26 -0700 |
---|---|---|
committer | Chris Broadfoot <cbro@golang.org> | 2016-09-07 18:48:24 +0000 |
commit | 24f46bd34fff99cbf64762b65e7896fd1e8f2f7c (patch) | |
tree | ad90c71defbcca9131689ffc42deec6f12d0831c | |
parent | c24b5d43a6cce1f25b141d37c7cd6a9f2c7d352c (diff) | |
download | go-24f46bd34fff99cbf64762b65e7896fd1e8f2f7c.tar.gz go-24f46bd34fff99cbf64762b65e7896fd1e8f2f7c.zip |
[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>
-rw-r--r-- | src/cmd/compile/internal/ssa/deadstore.go | 17 | ||||
-rw-r--r-- | src/cmd/compile/internal/ssa/deadstore_test.go | 33 |
2 files changed, 41 insertions, 9 deletions
diff --git a/src/cmd/compile/internal/ssa/deadstore.go b/src/cmd/compile/internal/ssa/deadstore.go index 5129c171bb..59ac1b9ad4 100644 --- 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 c38f1cdbaf..beb6de0637 100644 --- 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) + } +} |