cmd/compile: handle constant pointer offsets in dead store elimination

randall77 · randall77 · commit 43b57b85160f · 2023-10-31T20:42:56.000Z
Update #63657 Update #45573 Change-Id: I163c6038c13d974dc0ca9f02144472bc05331826 Reviewed-on: https://go-review.googlesource.com/c/go/+/538595 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: David Chase <drchase@google.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
@@ -73,9 +73,9 @@ func dse(f *Func) {
 		}
 
 		// Walk backwards looking for dead stores. Keep track of shadowed addresses.
-		// A "shadowed address" is a pointer and a size describing a memory region that
-		// is known to be written. We keep track of shadowed addresses in the shadowed
-		// map, mapping the ID of the address to the size of the shadowed region.
+		// A "shadowed address" is a pointer, offset, and size describing a memory region that
+		// is known to be written. We keep track of shadowed addresses in the shadowed map,
+		// mapping the ID of the address to a shadowRange where future writes will happen.
 		// Since we're walking backwards, writes to a shadowed region are useless,
 		// as they will be immediately overwritten.
 		shadowed.clear()
@@ -88,13 +88,20 @@ func dse(f *Func) {
 			shadowed.clear()
 		}
 		if v.Op == OpStore || v.Op == OpZero {
+			ptr := v.Args[0]
+			var off int64
+			for ptr.Op == OpOffPtr { // Walk to base pointer
+				off += ptr.AuxInt
+				ptr = ptr.Args[0]
+			}
 			var sz int64
 			if v.Op == OpStore {
 				sz = v.Aux.(*types.Type).Size()
 			} else { // OpZero
 				sz = v.AuxInt
 			}
-			if shadowedSize := int64(shadowed.get(v.Args[0].ID)); shadowedSize != -1 && shadowedSize >= sz {
+			sr := shadowRange(shadowed.get(ptr.ID))
+			if sr.contains(off, off+sz) {
 				// Modify the store/zero into a copy of the memory state,
 				// effectively eliding the store operation.
 				if v.Op == OpStore {
@@ -108,10 +115,8 @@ func dse(f *Func) {
 				v.AuxInt = 0
 				v.Op = OpCopy
 			} else {
-				if sz > 0x7fffffff { // work around sparseMap's int32 value type
-					sz = 0x7fffffff
-				}
-				shadowed.set(v.Args[0].ID, int32(sz))
+				// Extend shadowed region.
+				shadowed.set(ptr.ID, int32(sr.merge(off, off+sz)))
 			}
 		}
 		// walk to previous store
@@ -131,6 +136,49 @@ func dse(f *Func) {
 	}
 }
 
+// A shadowRange encodes a set of byte offsets [lo():hi()] from
+// a given pointer that will be written to later in the block.
+// A zero shadowRange encodes an empty shadowed range (and so
+// does a -1 shadowRange, which is what sparsemap.get returns
+// on a failed lookup).
+type shadowRange int32
+
+func (sr shadowRange) lo() int64 {
+	return int64(sr & 0xffff)
+}
+func (sr shadowRange) hi() int64 {
+	return int64((sr >> 16) & 0xffff)
+}
+
+// contains reports whether [lo:hi] is completely within sr.
+func (sr shadowRange) contains(lo, hi int64) bool {
+	return lo >= sr.lo() && hi <= sr.hi()
+}
+
+// merge returns the union of sr and [lo:hi].
+// merge is allowed to return something smaller than the union.
+func (sr shadowRange) merge(lo, hi int64) shadowRange {
+	if lo < 0 || hi > 0xffff {
+		// Ignore offsets that are too large or small.
+		return sr
+	}
+	if sr.lo() == sr.hi() {
+		// Old range is empty - use new one.
+		return shadowRange(lo + hi<<16)
+	}
+	if hi < sr.lo() || lo > sr.hi() {
+		// The two regions don't overlap or abut, so we would
+		// have to keep track of multiple disjoint ranges.
+		// Because we can only keep one, keep the larger one.
+		if sr.hi()-sr.lo() >= hi-lo {
+			return sr
+		}
+		return shadowRange(lo + hi<<16)
+	}
+	// Regions overlap or abut - compute the union.
+	return shadowRange(min(lo, sr.lo()) + max(hi, sr.hi())<<16)
+}
+
 // elimDeadAutosGeneric deletes autos that are never accessed. To achieve this
 // we track the operations that the address of each auto reaches and if it only
 // reaches stores then we delete all the stores. The other operations will then
diff --git a/src/cmd/compile/internal/ssa/rewrite.go b/src/cmd/compile/internal/ssa/rewrite.go
@@ -1183,6 +1183,12 @@ func min(x, y int64) int64 {
 	}
 	return y
 }
+func max(x, y int64) int64 {
+	if x > y {
+		return x
+	}
+	return y
+}
 
 func isConstZero(v *Value) bool {
 	switch v.Op {

Original file line number	Diff line number	Diff line change
`@@ -1183,6 +1183,12 @@ func min(x, y int64) int64 {`
`1183`	`1183`	`}`
`1184`	`1184`	`return y`
`1185`	`1185`	`}`
	`1186`	`+func max(x, y int64) int64 {`
	`1187`	`+ if x > y {`
	`1188`	`+ return x`
	`1189`	`+ }`
	`1190`	`+ return y`
	`1191`	`+}`
`1186`	`1192`
`1187`	`1193`	`func isConstZero(v *Value) bool {`
`1188`	`1194`	`switch v.Op {`