From: khr@golang.org Date: Fri, 19 Jul 2024 21:54:54 +0000 (-0700) Subject: cmd/compile: rip out constant handling in poset data structure X-Git-Tag: go1.24rc1~1253 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=b97971ea2edbc29798e8a29a5e270698987a7f1d;p=gostls13.git cmd/compile: rip out constant handling in poset data structure The prove pass now tracks possible constant values explicitly, so the poset data structure no longer has to. This simplifies a bunch of the special cases in poset. Change-Id: I0efff65269bc5d53c6d18e4760b0375cfb2ae8b7 Reviewed-on: https://go-review.googlesource.com/c/go/+/599795 Reviewed-by: David Chase LUCI-TryBot-Result: Go LUCI Reviewed-by: Michael Knyszek --- diff --git a/src/cmd/compile/internal/ssa/poset.go b/src/cmd/compile/internal/ssa/poset.go index 7b64843fe9..50b4d17889 100644 --- a/src/cmd/compile/internal/ssa/poset.go +++ b/src/cmd/compile/internal/ssa/poset.go @@ -42,17 +42,16 @@ func (bs bitset) Test(idx uint32) bool { type undoType uint8 const ( - undoInvalid undoType = iota - undoCheckpoint // a checkpoint to group undo passes - undoSetChl // change back left child of undo.idx to undo.edge - undoSetChr // change back right child of undo.idx to undo.edge - undoNonEqual // forget that SSA value undo.ID is non-equal to undo.idx (another ID) - undoNewNode // remove new node created for SSA value undo.ID - undoNewConstant // remove the constant node idx from the constants map - undoAliasNode // unalias SSA value undo.ID so that it points back to node index undo.idx - undoNewRoot // remove node undo.idx from root list - undoChangeRoot // remove node undo.idx from root list, and put back undo.edge.Target instead - undoMergeRoot // remove node undo.idx from root list, and put back its children instead + undoInvalid undoType = iota + undoCheckpoint // a checkpoint to group undo passes + undoSetChl // change back left child of undo.idx to undo.edge + undoSetChr // change back right child of undo.idx to undo.edge + undoNonEqual // forget that SSA value undo.ID is non-equal to undo.idx (another ID) + undoNewNode // remove new node created for SSA value undo.ID + undoAliasNode // unalias SSA value undo.ID so that it points back to node index undo.idx + undoNewRoot // remove node undo.idx from root list + undoChangeRoot // remove node undo.idx from root list, and put back undo.edge.Target instead + undoMergeRoot // remove node undo.idx from root list, and put back its children instead ) // posetUndo represents an undo pass to be performed. @@ -67,7 +66,8 @@ type posetUndo struct { } const ( - // Make poset handle constants as unsigned numbers. + // Make poset handle values as unsigned numbers. + // (TODO: remove?) posetFlagUnsigned = 1 << iota ) @@ -124,14 +124,6 @@ type posetNode struct { // two SSA values to the same DAG node; when a new equality relation is recorded // between two existing nodes, the nodes are merged, adjusting incoming and outgoing edges. // -// Constants are specially treated. When a constant is added to the poset, it is -// immediately linked to other constants already present; so for instance if the -// poset knows that x<=3, and then x is tested against 5, 5 is first added and linked -// 3 (using 3<5), so that the poset knows that x<=3<5; at that point, it is able -// to answer x<5 correctly. This means that all constants are always within the same -// DAG; as an implementation detail, we enfoce that the DAG containtining the constants -// is always the first in the forest. -// // poset is designed to be memory efficient and do little allocations during normal usage. // Most internal data structures are pre-allocated and flat, so for instance adding a // new relation does not cause any allocation. For performance reasons, @@ -146,24 +138,22 @@ type posetNode struct { // / \ // J K type poset struct { - lastidx uint32 // last generated dense index - flags uint8 // internal flags - values map[ID]uint32 // map SSA values to dense indexes - constants map[int64]uint32 // record SSA constants together with their value - nodes []posetNode // nodes (in all DAGs) - roots []uint32 // list of root nodes (forest) - noneq map[uint32]bitset // non-equal relations - undo []posetUndo // undo chain + lastidx uint32 // last generated dense index + flags uint8 // internal flags + values map[ID]uint32 // map SSA values to dense indexes + nodes []posetNode // nodes (in all DAGs) + roots []uint32 // list of root nodes (forest) + noneq map[uint32]bitset // non-equal relations + undo []posetUndo // undo chain } func newPoset() *poset { return &poset{ - values: make(map[ID]uint32), - constants: make(map[int64]uint32, 8), - nodes: make([]posetNode, 1, 16), - roots: make([]uint32, 0, 4), - noneq: make(map[uint32]bitset), - undo: make([]posetUndo, 0, 4), + values: make(map[ID]uint32), + nodes: make([]posetNode, 1, 16), + roots: make([]uint32, 0, 4), + noneq: make(map[uint32]bitset), + undo: make([]posetUndo, 0, 4), } } @@ -205,11 +195,6 @@ func (po *poset) upushalias(id ID, i2 uint32) { po.undo = append(po.undo, posetUndo{typ: undoAliasNode, ID: id, idx: i2}) } -// upushconst pushes a new undo pass for a new constant -func (po *poset) upushconst(idx uint32, old uint32) { - po.undo = append(po.undo, posetUndo{typ: undoNewConstant, idx: idx, ID: ID(old)}) -} - // addchild adds i2 as direct child of i1. func (po *poset) addchild(i1, i2 uint32, strict bool) { i1l, i1r := po.children(i1) @@ -268,144 +253,11 @@ func (po *poset) newnode(n *Value) uint32 { } // lookup searches for a SSA value into the forest of DAGS, and return its node. -// Constants are materialized on the fly during lookup. func (po *poset) lookup(n *Value) (uint32, bool) { i, f := po.values[n.ID] - if !f && n.isGenericIntConst() { - po.newconst(n) - i, f = po.values[n.ID] - } return i, f } -// newconst creates a node for a constant. It links it to other constants, so -// that n<=5 is detected true when n<=3 is known to be true. -// TODO: this is O(N), fix it. -func (po *poset) newconst(n *Value) { - if !n.isGenericIntConst() { - panic("newconst on non-constant") - } - - // If the same constant is already present in the poset through a different - // Value, just alias to it without allocating a new node. - val := n.AuxInt - if po.flags&posetFlagUnsigned != 0 { - val = int64(n.AuxUnsigned()) - } - if c, found := po.constants[val]; found { - po.values[n.ID] = c - po.upushalias(n.ID, 0) - return - } - - // Create the new node for this constant - i := po.newnode(n) - - // If this is the first constant, put it as a new root, as - // we can't record an existing connection so we don't have - // a specific DAG to add it to. Notice that we want all - // constants to be in root #0, so make sure the new root - // goes there. - if len(po.constants) == 0 { - idx := len(po.roots) - po.roots = append(po.roots, i) - po.roots[0], po.roots[idx] = po.roots[idx], po.roots[0] - po.upush(undoNewRoot, i, 0) - po.constants[val] = i - po.upushconst(i, 0) - return - } - - // Find the lower and upper bound among existing constants. That is, - // find the higher constant that is lower than the one that we're adding, - // and the lower constant that is higher. - // The loop is duplicated to handle signed and unsigned comparison, - // depending on how the poset was configured. - var lowerptr, higherptr uint32 - - if po.flags&posetFlagUnsigned != 0 { - var lower, higher uint64 - val1 := n.AuxUnsigned() - for val2, ptr := range po.constants { - val2 := uint64(val2) - if val1 == val2 { - panic("unreachable") - } - if val2 < val1 && (lowerptr == 0 || val2 > lower) { - lower = val2 - lowerptr = ptr - } else if val2 > val1 && (higherptr == 0 || val2 < higher) { - higher = val2 - higherptr = ptr - } - } - } else { - var lower, higher int64 - val1 := n.AuxInt - for val2, ptr := range po.constants { - if val1 == val2 { - panic("unreachable") - } - if val2 < val1 && (lowerptr == 0 || val2 > lower) { - lower = val2 - lowerptr = ptr - } else if val2 > val1 && (higherptr == 0 || val2 < higher) { - higher = val2 - higherptr = ptr - } - } - } - - if lowerptr == 0 && higherptr == 0 { - // This should not happen, as at least one - // other constant must exist if we get here. - panic("no constant found") - } - - // Create the new node and connect it to the bounds, so that - // lower < n < higher. We could have found both bounds or only one - // of them, depending on what other constants are present in the poset. - // Notice that we always link constants together, so they - // are always part of the same DAG. - switch { - case lowerptr != 0 && higherptr != 0: - // Both bounds are present, record lower < n < higher. - po.addchild(lowerptr, i, true) - po.addchild(i, higherptr, true) - - case lowerptr != 0: - // Lower bound only, record lower < n. - po.addchild(lowerptr, i, true) - - case higherptr != 0: - // Higher bound only. To record n < higher, we need - // an extra root: - // - // extra - // / \ - // root \ - // / n - // .... / - // \ / - // higher - // - i2 := higherptr - r2 := po.findroot(i2) - if r2 != po.roots[0] { // all constants should be in root #0 - panic("constant not in root #0") - } - extra := po.newnode(nil) - po.changeroot(r2, extra) - po.upush(undoChangeRoot, extra, newedge(r2, false)) - po.addchild(extra, r2, false) - po.addchild(extra, i, false) - po.addchild(i, i2, true) - } - - po.constants[val] = i - po.upushconst(i, 0) -} - // aliasnewnode records that a single node n2 (not in the poset yet) is an alias // of the master node n1. func (po *poset) aliasnewnode(n1, n2 *Value) { @@ -474,15 +326,6 @@ func (po *poset) aliasnodes(n1 *Value, i2s bitset) { po.upushalias(k, v) } } - - // If one of the aliased nodes is a constant, then make sure - // po.constants is updated to point to the master node. - for val, idx := range po.constants { - if i2s.Test(idx) { - po.constants[val] = i1 - po.upushconst(i1, idx) - } - } } func (po *poset) isroot(r uint32) bool { @@ -613,12 +456,6 @@ func (po *poset) findroot(i uint32) uint32 { // mergeroot merges two DAGs into one DAG by creating a new extra root func (po *poset) mergeroot(r1, r2 uint32) uint32 { - // Root #0 is special as it contains all constants. Since mergeroot - // discards r2 as root and keeps r1, make sure that r2 is not root #0, - // otherwise constants would move to a different root. - if r2 == po.roots[0] { - r1, r2 = r2, r1 - } r := po.newnode(nil) po.setchl(r, newedge(r1, false)) po.setchr(r, newedge(r2, false)) @@ -738,16 +575,9 @@ func (po *poset) setnoneq(n1, n2 *Value) { // CheckIntegrity verifies internal integrity of a poset. It is intended // for debugging purposes. func (po *poset) CheckIntegrity() { - // Record which index is a constant - constants := newBitset(int(po.lastidx + 1)) - for _, c := range po.constants { - constants.Set(c) - } - - // Verify that each node appears in a single DAG, and that - // all constants are within the first DAG + // Verify that each node appears in a single DAG seen := newBitset(int(po.lastidx + 1)) - for ridx, r := range po.roots { + for _, r := range po.roots { if r == 0 { panic("empty root") } @@ -757,11 +587,6 @@ func (po *poset) CheckIntegrity() { panic("duplicate node") } seen.Set(i) - if constants.Test(i) { - if ridx != 0 { - panic("constants not in the first DAG") - } - } return false }) } @@ -799,9 +624,6 @@ func (po *poset) CheckEmpty() error { if len(po.roots) != 0 { return fmt.Errorf("non-empty root list: %v", po.roots) } - if len(po.constants) != 0 { - return fmt.Errorf("non-empty constants: %v", po.constants) - } if len(po.undo) != 0 { return fmt.Errorf("non-empty undo list: %v", po.undo) } @@ -838,31 +660,12 @@ func (po *poset) DotDump(fn string, title string) error { names[i] = s } - // Create reverse constant mapping - consts := make(map[uint32]int64) - for val, idx := range po.constants { - consts[idx] = val - } - fmt.Fprintf(f, "digraph poset {\n") fmt.Fprintf(f, "\tedge [ fontsize=10 ]\n") for ridx, r := range po.roots { fmt.Fprintf(f, "\tsubgraph root%d {\n", ridx) po.dfs(r, false, func(i uint32) bool { - if val, ok := consts[i]; ok { - // Constant - var vals string - if po.flags&posetFlagUnsigned != 0 { - vals = fmt.Sprint(uint64(val)) - } else { - vals = fmt.Sprint(int64(val)) - } - fmt.Fprintf(f, "\t\tnode%d [shape=box style=filled fillcolor=cadetblue1 label=<%s %s [%d]>]\n", - i, vals, names[i], i) - } else { - // Normal SSA value - fmt.Fprintf(f, "\t\tnode%d [label=<%s [%d]>]\n", i, names[i], i) - } + fmt.Fprintf(f, "\t\tnode%d [label=<%s [%d]>]\n", i, names[i], i) chl, chr := po.children(i) for _, ch := range []posetEdge{chl, chr} { if ch != 0 { @@ -1290,27 +1093,6 @@ func (po *poset) Undo() { po.nodes = po.nodes[:pass.idx] po.lastidx-- - case undoNewConstant: - // FIXME: remove this O(n) loop - var val int64 - var i uint32 - for val, i = range po.constants { - if i == pass.idx { - break - } - } - if i != pass.idx { - panic("constant not found in undo pass") - } - if pass.ID == 0 { - delete(po.constants, val) - } else { - // Restore previous index as constant node - // (also restoring the invariant on correct bounds) - oldidx := uint32(pass.ID) - po.constants[val] = oldidx - } - case undoAliasNode: ID, prev := pass.ID, pass.idx cur := po.values[ID] diff --git a/src/cmd/compile/internal/ssa/poset_test.go b/src/cmd/compile/internal/ssa/poset_test.go index a6db1d1c24..17918f2550 100644 --- a/src/cmd/compile/internal/ssa/poset_test.go +++ b/src/cmd/compile/internal/ssa/poset_test.go @@ -42,13 +42,6 @@ func vconst(i int) int { return 1000 + 128 + i } -func vconst2(i int) int { - if i < -128 || i >= 128 { - panic("invalid const") - } - return 1000 + 256 + i -} - func testPosetOps(t *testing.T, unsigned bool, ops []posetTestOp) { var v [1512]*Value for i := range v { @@ -58,10 +51,6 @@ func testPosetOps(t *testing.T, unsigned bool, ops []posetTestOp) { v[i].Op = OpConst64 v[i].AuxInt = int64(i - 1000 - 128) } - if i >= 1256 && i < 1512 { - v[i].Op = OpConst64 - v[i].AuxInt = int64(i - 1000 - 256) - } } po := newPoset() @@ -478,7 +467,6 @@ func TestPosetCollapse(t *testing.T) { {Equal, 10, 18}, {Equal, 10, 19}, {Equal, 10, vconst(20)}, - {Equal, 10, vconst2(20)}, {Equal, 10, 25}, {Equal, 12, 15}, @@ -487,7 +475,6 @@ func TestPosetCollapse(t *testing.T) { {Equal, 12, 18}, {Equal, 12, 19}, {Equal, 12, vconst(20)}, - {Equal, 12, vconst2(20)}, {Equal, 12, 25}, {Equal, 15, 16}, @@ -495,36 +482,28 @@ func TestPosetCollapse(t *testing.T) { {Equal, 15, 18}, {Equal, 15, 19}, {Equal, 15, vconst(20)}, - {Equal, 15, vconst2(20)}, {Equal, 15, 25}, {Equal, 16, 17}, {Equal, 16, 18}, {Equal, 16, 19}, {Equal, 16, vconst(20)}, - {Equal, 16, vconst2(20)}, {Equal, 16, 25}, {Equal, 17, 18}, {Equal, 17, 19}, {Equal, 17, vconst(20)}, - {Equal, 17, vconst2(20)}, {Equal, 17, 25}, {Equal, 18, 19}, {Equal, 18, vconst(20)}, - {Equal, 18, vconst2(20)}, {Equal, 18, 25}, {Equal, 19, vconst(20)}, - {Equal, 19, vconst2(20)}, {Equal, 19, 25}, - {Equal, vconst(20), vconst2(20)}, {Equal, vconst(20), 25}, - {Equal, vconst2(20), 25}, - // ... but not 11/26/100/101/102, which were on a different path {Equal_Fail, 10, 11}, {Equal_Fail, 10, 26}, @@ -632,117 +611,6 @@ func TestPosetSetEqual(t *testing.T) { }) } -func TestPosetConst(t *testing.T) { - testPosetOps(t, false, []posetTestOp{ - {Checkpoint, 0, 0}, - {SetOrder, 1, vconst(15)}, - {SetOrderOrEqual, 100, vconst(120)}, - {Ordered, 1, vconst(15)}, - {Ordered, 1, vconst(120)}, - {OrderedOrEqual, 1, vconst(120)}, - {OrderedOrEqual, 100, vconst(120)}, - {Ordered_Fail, 100, vconst(15)}, - {Ordered_Fail, vconst(15), 100}, - - {Checkpoint, 0, 0}, - {SetOrderOrEqual, 1, 5}, - {SetOrderOrEqual, 5, 25}, - {SetEqual, 20, vconst(20)}, - {SetEqual, 25, vconst(25)}, - {Ordered, 1, 20}, - {Ordered, 1, vconst(30)}, - {Undo, 0, 0}, - - {Checkpoint, 0, 0}, - {SetOrderOrEqual, 1, 5}, - {SetOrderOrEqual, 5, 25}, - {SetEqual, vconst(-20), 5}, - {SetEqual, vconst(-25), 1}, - {Ordered, 1, 5}, - {Ordered, vconst(-30), 1}, - {Undo, 0, 0}, - - {Checkpoint, 0, 0}, - {SetNonEqual, 1, vconst(4)}, - {SetNonEqual, 1, vconst(6)}, - {NonEqual, 1, vconst(4)}, - {NonEqual_Fail, 1, vconst(5)}, - {NonEqual, 1, vconst(6)}, - {Equal_Fail, 1, vconst(4)}, - {Equal_Fail, 1, vconst(5)}, - {Equal_Fail, 1, vconst(6)}, - {Equal_Fail, 1, vconst(7)}, - {Undo, 0, 0}, - - {Undo, 0, 0}, - }) - - testPosetOps(t, true, []posetTestOp{ - {Checkpoint, 0, 0}, - {SetOrder, 1, vconst(15)}, - {SetOrderOrEqual, 100, vconst(-5)}, // -5 is a very big number in unsigned - {Ordered, 1, vconst(15)}, - {Ordered, 1, vconst(-5)}, - {OrderedOrEqual, 1, vconst(-5)}, - {OrderedOrEqual, 100, vconst(-5)}, - {Ordered_Fail, 100, vconst(15)}, - {Ordered_Fail, vconst(15), 100}, - - {Undo, 0, 0}, - }) - - testPosetOps(t, false, []posetTestOp{ - {Checkpoint, 0, 0}, - {SetOrderOrEqual, 1, vconst(3)}, - {SetNonEqual, 1, vconst(0)}, - {Ordered_Fail, 1, vconst(0)}, - {Undo, 0, 0}, - }) - - testPosetOps(t, false, []posetTestOp{ - // Check relations of a constant with itself - {Checkpoint, 0, 0}, - {SetOrderOrEqual, vconst(3), vconst2(3)}, - {Undo, 0, 0}, - {Checkpoint, 0, 0}, - {SetEqual, vconst(3), vconst2(3)}, - {Undo, 0, 0}, - {Checkpoint, 0, 0}, - {SetNonEqual_Fail, vconst(3), vconst2(3)}, - {Undo, 0, 0}, - {Checkpoint, 0, 0}, - {SetOrder_Fail, vconst(3), vconst2(3)}, - {Undo, 0, 0}, - - // Check relations of two constants among them, using - // different instances of the same constant - {Checkpoint, 0, 0}, - {SetOrderOrEqual, vconst(3), vconst(4)}, - {OrderedOrEqual, vconst(3), vconst2(4)}, - {Undo, 0, 0}, - {Checkpoint, 0, 0}, - {SetOrder, vconst(3), vconst(4)}, - {Ordered, vconst(3), vconst2(4)}, - {Undo, 0, 0}, - {Checkpoint, 0, 0}, - {SetEqual_Fail, vconst(3), vconst(4)}, - {SetEqual_Fail, vconst(3), vconst2(4)}, - {Undo, 0, 0}, - {Checkpoint, 0, 0}, - {NonEqual, vconst(3), vconst(4)}, - {NonEqual, vconst(3), vconst2(4)}, - {Undo, 0, 0}, - {Checkpoint, 0, 0}, - {Equal_Fail, vconst(3), vconst(4)}, - {Equal_Fail, vconst(3), vconst2(4)}, - {Undo, 0, 0}, - {Checkpoint, 0, 0}, - {SetNonEqual, vconst(3), vconst(4)}, - {SetNonEqual, vconst(3), vconst2(4)}, - {Undo, 0, 0}, - }) -} - func TestPosetNonEqual(t *testing.T) { testPosetOps(t, false, []posetTestOp{ {Equal_Fail, 10, 20},