]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/compile: in poset, make constant handling more flexible
authorGiovanni Bajo <rasky@develer.com>
Sat, 21 Sep 2019 23:26:50 +0000 (01:26 +0200)
committerGiovanni Bajo <rasky@develer.com>
Mon, 14 Oct 2019 21:27:08 +0000 (21:27 +0000)
Currently, constants in posets, in addition to being stored in
a DAG, are also stored as SSA values in a slice. This allows to
quickly go through all stored constants, but it's not easy to search
for a specific constant.

Following CLs will benefit from being able to quickly find
a constants by value in the poset, so change the constants
structure to a map. Since we're at it, don't store it as
*ssa.Value: poset always uses dense uint32 indices when
referring a node, so just switch to it.

Using a map also forces us to have a single node per
constant value: this is a good thing in the first place,
so this CL also make sure we never create two nodes for
the same constant value.

Change-Id: I099814578af35f935ebf14bc4767d607021f5f8b
Reviewed-on: https://go-review.googlesource.com/c/go/+/196781
Run-TryBot: Giovanni Bajo <rasky@develer.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
src/cmd/compile/internal/ssa/poset.go

index cf5b915b9460561f8b136e8c47ced9db5c4a2a87..1ddc3e3277b46b31cc1c5729d32cb7f36dd2de55 100644 (file)
@@ -42,16 +42,17 @@ 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
-       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
+       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
 )
 
 // posetUndo represents an undo pass to be performed.
@@ -146,20 +147,20 @@ 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 []*Value      // record SSA constants together with their value
-       nodes     []posetNode   // nodes (in all DAGs)
-       roots     []uint32      // list of root nodes (forest)
-       noneq     map[ID]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
+       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[ID]bitset    // non-equal relations
+       undo      []posetUndo      // undo chain
 }
 
 func newPoset() *poset {
        return &poset{
                values:    make(map[ID]uint32),
-               constants: make([]*Value, 0, 8),
+               constants: make(map[int64]uint32, 8),
                nodes:     make([]posetNode, 1, 16),
                roots:     make([]uint32, 0, 4),
                noneq:     make(map[ID]bitset),
@@ -205,6 +206,11 @@ 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)
@@ -281,18 +287,33 @@ func (po *poset) newconst(n *Value) {
                panic("newconst on non-constant")
        }
 
-       // If this is the first constant, put it into a new root, as
+       // 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)
-               i := po.newnode(n)
                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 = append(po.constants, n)
+               po.constants[val] = i
+               po.upushconst(i, 0)
                return
        }
 
@@ -301,21 +322,20 @@ func (po *poset) newconst(n *Value) {
        // 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 *Value
+       var lowerptr, higherptr uint32
 
        if po.flags&posetFlagUnsigned != 0 {
                var lower, higher uint64
                val1 := n.AuxUnsigned()
-               for _, ptr := range po.constants {
-                       val2 := ptr.AuxUnsigned()
+               for val2, ptr := range po.constants {
+                       val2 := uint64(val2)
                        if val1 == val2 {
-                               po.aliasnode(ptr, n)
-                               return
+                               panic("unreachable")
                        }
-                       if val2 < val1 && (lowerptr == nil || val2 > lower) {
+                       if val2 < val1 && (lowerptr == 0 || val2 > lower) {
                                lower = val2
                                lowerptr = ptr
-                       } else if val2 > val1 && (higherptr == nil || val2 < higher) {
+                       } else if val2 > val1 && (higherptr == 0 || val2 < higher) {
                                higher = val2
                                higherptr = ptr
                        }
@@ -323,23 +343,21 @@ func (po *poset) newconst(n *Value) {
        } else {
                var lower, higher int64
                val1 := n.AuxInt
-               for _, ptr := range po.constants {
-                       val2 := ptr.AuxInt
+               for val2, ptr := range po.constants {
                        if val1 == val2 {
-                               po.aliasnode(ptr, n)
-                               return
+                               panic("unreachable")
                        }
-                       if val2 < val1 && (lowerptr == nil || val2 > lower) {
+                       if val2 < val1 && (lowerptr == 0 || val2 > lower) {
                                lower = val2
                                lowerptr = ptr
-                       } else if val2 > val1 && (higherptr == nil || val2 < higher) {
+                       } else if val2 > val1 && (higherptr == 0 || val2 < higher) {
                                higher = val2
                                higherptr = ptr
                        }
                }
        }
 
-       if lowerptr == nil && higherptr == nil {
+       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")
@@ -350,18 +368,17 @@ func (po *poset) newconst(n *Value) {
        // 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.
-       i := po.newnode(n)
        switch {
-       case lowerptr != nil && higherptr != nil:
+       case lowerptr != 0 && higherptr != 0:
                // Both bounds are present, record lower < n < higher.
-               po.addchild(po.values[lowerptr.ID], i, true)
-               po.addchild(i, po.values[higherptr.ID], true)
+               po.addchild(lowerptr, i, true)
+               po.addchild(i, higherptr, true)
 
-       case lowerptr != nil:
+       case lowerptr != 0:
                // Lower bound only, record lower < n.
-               po.addchild(po.values[lowerptr.ID], i, true)
+               po.addchild(lowerptr, i, true)
 
-       case higherptr != nil:
+       case higherptr != 0:
                // Higher bound only. To record n < higher, we need
                // a dummy root:
                //
@@ -373,7 +390,7 @@ func (po *poset) newconst(n *Value) {
                //       \    /
                //       higher
                //
-               i2 := po.values[higherptr.ID]
+               i2 := higherptr
                r2 := po.findroot(i2)
                if r2 != po.roots[0] { // all constants should be in root #0
                        panic("constant not in root #0")
@@ -386,7 +403,8 @@ func (po *poset) newconst(n *Value) {
                po.addchild(i, i2, true)
        }
 
-       po.constants = append(po.constants, n)
+       po.constants[val] = i
+       po.upushconst(i, 0)
 }
 
 // aliasnode records that n2 is an alias of n1
@@ -422,6 +440,19 @@ func (po *poset) aliasnode(n1, n2 *Value) {
                                po.upushalias(k, i2)
                        }
                }
+
+               if n2.isGenericIntConst() {
+                       val := n2.AuxInt
+                       if po.flags&posetFlagUnsigned != 0 {
+                               val = int64(n2.AuxUnsigned())
+                       }
+                       if po.constants[val] != i2 {
+                               panic("aliasing constant which is not registered")
+                       }
+                       po.constants[val] = i1
+                       po.upushconst(i1, i2)
+               }
+
        } else {
                // n2.ID wasn't seen before, so record it as alias to i1
                po.values[n2.ID] = i1
@@ -631,11 +662,7 @@ func (po *poset) CheckIntegrity() {
        // Record which index is a constant
        constants := newBitset(int(po.lastidx + 1))
        for _, c := range po.constants {
-               if idx, ok := po.values[c.ID]; !ok {
-                       panic("node missing for constant")
-               } else {
-                       constants.Set(idx)
-               }
+               constants.Set(c)
        }
 
        // Verify that each node appears in a single DAG, and that
@@ -732,15 +759,10 @@ func (po *poset) DotDump(fn string, title string) error {
                names[i] = s
        }
 
-       // Create constant mapping
+       // Create reverse constant mapping
        consts := make(map[uint32]int64)
-       for _, v := range po.constants {
-               idx := po.values[v.ID]
-               if po.flags&posetFlagUnsigned != 0 {
-                       consts[idx] = int64(v.AuxUnsigned())
-               } else {
-                       consts[idx] = v.AuxInt
-               }
+       for val, idx := range po.constants {
+               consts[idx] = val
        }
 
        fmt.Fprintf(f, "digraph poset {\n")
@@ -1171,10 +1193,25 @@ func (po *poset) Undo() {
                        po.nodes = po.nodes[:pass.idx]
                        po.lastidx--
 
-                       // If it was the last inserted constant, remove it
-                       nc := len(po.constants)
-                       if nc > 0 && po.constants[nc-1].ID == pass.ID {
-                               po.constants = po.constants[:nc-1]
+               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: