Commit 24f46bd3 authored by Keith Randall's avatar Keith Randall Committed by Chris Broadfoot

[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/28670Reviewed-by: default avatarCherry Zhang <cherryyz@google.com>
Reviewed-by: default avatarDavid Chase <drchase@google.com>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
parent c24b5d43
......@@ -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
......
......@@ -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)
}
}
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment