]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/compile: rip out constant handling in poset data structure
authorkhr@golang.org <khr@golang.org>
Fri, 19 Jul 2024 21:54:54 +0000 (14:54 -0700)
committerKeith Randall <khr@golang.org>
Wed, 7 Aug 2024 16:08:28 +0000 (16:08 +0000)
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 <drchase@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
src/cmd/compile/internal/ssa/poset.go
src/cmd/compile/internal/ssa/poset_test.go

index 7b64843fe964d2fe38921e101b41aa49c69c68c9..50b4d1788929170dd9f3cc5eefc8d1ef18901640 100644 (file)
@@ -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 <font point-size=\"6\">%s [%d]</font>>]\n",
-                                       i, vals, names[i], i)
-                       } else {
-                               // Normal SSA value
-                               fmt.Fprintf(f, "\t\tnode%d [label=<%s <font point-size=\"6\">[%d]</font>>]\n", i, names[i], i)
-                       }
+                       fmt.Fprintf(f, "\t\tnode%d [label=<%s <font point-size=\"6\">[%d]</font>>]\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]
index a6db1d1c24a8cfc69e98aec56e75b132469ed7a7..17918f2550063b69ed0ca43b9f58781dfe17b5a2 100644 (file)
@@ -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},