From: Giovanni Bajo Date: Sat, 21 Sep 2019 23:26:50 +0000 (+0200) Subject: cmd/compile: in poset, make constant handling more flexible X-Git-Tag: go1.14beta1~750 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=c3a871fde17a33605600b1206904daa7f7d04bb3;p=gostls13.git cmd/compile: in poset, make constant handling more flexible 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 TryBot-Result: Gobot Gobot Reviewed-by: David Chase --- diff --git a/src/cmd/compile/internal/ssa/poset.go b/src/cmd/compile/internal/ssa/poset.go index cf5b915b94..1ddc3e3277 100644 --- a/src/cmd/compile/internal/ssa/poset.go +++ b/src/cmd/compile/internal/ssa/poset.go @@ -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: