Commit a7277e54 authored by Cherry Zhang's avatar Cherry Zhang

cmd/compile: compare size in dead store elimination

Only remove stores that is shadowed by another store with same or
larger size. Normally we don't need this check because we did check
the types, but unsafe pointer casting can get around it.

Fixes #16769.

Change-Id: I3f7c6c57807b590a2f735007dec6c65a4fa01a34
Reviewed-on: https://go-review.googlesource.com/27320
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: default avatarKeith Randall <khr@golang.org>
parent 5b9ff11c
......@@ -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,23 @@ func dse(f *Func) {
shadowed.clear()
}
if v.Op == OpStore || v.Op == OpZero {
if shadowed.contains(v.Args[0].ID) {
var sz int64
if v.Op == OpStore {
sz = v.AuxInt
} else { // OpZero
sz = SizeAndAlign(v.AuxInt).Size()
}
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 SizeAndAlign(v.AuxInt).Size() != 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 +104,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