From: Josh Bleecher Snyder Date: Fri, 17 Jun 2016 21:50:39 +0000 (-0700) Subject: cmd/compile: simplify constant switch case sorting X-Git-Tag: go1.8beta1~1704 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=a9266eef9384090d748acf723351a282b71982a8;p=gostls13.git cmd/compile: simplify constant switch case sorting This sort is now only reachable for constant clauses for a non-interface switch expression value. Refactor a bit so that the few tests that remain are concise and easy to read. Add a test that string length takes priority over built-in string order. Change-Id: Iedaa11ff77049d5ad1bf14f54cbb8c3411d589a7 Reviewed-on: https://go-review.googlesource.com/26767 Run-TryBot: Josh Bleecher Snyder Reviewed-by: Matthew Dempsky --- diff --git a/src/cmd/compile/internal/gc/swt.go b/src/cmd/compile/internal/gc/swt.go index c053099681..46e3950ac3 100644 --- a/src/cmd/compile/internal/gc/swt.go +++ b/src/cmd/compile/internal/gc/swt.go @@ -267,7 +267,7 @@ func (s *exprSwitch) walk(sw *Node) { } // sort and compile constants - sort.Sort(caseClauseByExpr(cc[:run])) + sort.Sort(caseClauseByConstVal(cc[:run])) a := s.walkCases(cc[:run]) cas = append(cas, a) cc = cc[run:] @@ -315,7 +315,7 @@ func (s *exprSwitch) walkCases(cc []caseClause) *Node { mid := cc[half-1].node.Left le := Nod(OLE, s.exprname, mid) if Isconst(mid, CTSTR) { - // Search by length and then by value; see exprcmp. + // Search by length and then by value; see caseClauseByConstVal. lenlt := Nod(OLT, Nod(OLEN, s.exprname, nil), Nod(OLEN, mid, nil)) leneq := Nod(OEQ, Nod(OLEN, s.exprname, nil), Nod(OLEN, mid, nil)) a.Left = Nod(OOROR, lenlt, Nod(OANDAND, leneq, le)) @@ -778,71 +778,35 @@ func (s *typeSwitch) walkCases(cc []caseClause) *Node { return a } -type caseClauseByExpr []caseClause - -func (x caseClauseByExpr) Len() int { return len(x) } -func (x caseClauseByExpr) Swap(i, j int) { x[i], x[j] = x[j], x[i] } -func (x caseClauseByExpr) Less(i, j int) bool { - return exprcmp(x[i], x[j]) < 0 -} - -func exprcmp(c1, c2 caseClause) int { - // sort non-constants last - if !c1.isconst { - return +1 - } - if !c2.isconst { - return -1 - } - - n1 := c1.node.Left - n2 := c2.node.Left - - // sort by type (for switches on interface) - ct := n1.Val().Ctype() - if ct > n2.Val().Ctype() { - return +1 - } - if ct < n2.Val().Ctype() { - return -1 - } - if !Eqtype(n1.Type, n2.Type) { - if n1.Type.Vargen > n2.Type.Vargen { - return +1 - } else { - return -1 - } - } - - // sort by constant value to enable binary search - switch ct { - case CTFLT: - return n1.Val().U.(*Mpflt).Cmp(n2.Val().U.(*Mpflt)) - case CTINT, CTRUNE: - return n1.Val().U.(*Mpint).Cmp(n2.Val().U.(*Mpint)) - case CTSTR: +// caseClauseByConstVal sorts clauses by constant value to enable binary search. +type caseClauseByConstVal []caseClause + +func (x caseClauseByConstVal) Len() int { return len(x) } +func (x caseClauseByConstVal) Swap(i, j int) { x[i], x[j] = x[j], x[i] } +func (x caseClauseByConstVal) Less(i, j int) bool { + v1 := x[i].node.Left.Val().U + v2 := x[j].node.Left.Val().U + + switch v1 := v1.(type) { + case *Mpflt: + return v1.Cmp(v2.(*Mpflt)) < 0 + case *Mpint: + return v1.Cmp(v2.(*Mpint)) < 0 + case string: // Sort strings by length and then by value. // It is much cheaper to compare lengths than values, // and all we need here is consistency. // We respect this sorting in exprSwitch.walkCases. - a := n1.Val().U.(string) - b := n2.Val().U.(string) - if len(a) < len(b) { - return -1 - } - if len(a) > len(b) { - return +1 - } - if a == b { - return 0 - } - if a < b { - return -1 + a := v1 + b := v2.(string) + if len(a) != len(b) { + return len(a) < len(b) } - return +1 + return a < b } - return 0 + Fatalf("caseClauseByConstVal passed bad clauses %v < %v", x[i].node.Left, x[j].node.Left) + return false } type caseClauseByType []caseClause diff --git a/src/cmd/compile/internal/gc/swt_test.go b/src/cmd/compile/internal/gc/swt_test.go index 332433d31d..8b86df6616 100644 --- a/src/cmd/compile/internal/gc/swt_test.go +++ b/src/cmd/compile/internal/gc/swt_test.go @@ -9,136 +9,38 @@ import ( "testing" ) -func TestExprcmp(t *testing.T) { - testdata := []struct { - a, b caseClause - want int +func nodrune(r rune) *Node { + return nodlit(Val{&Mpint{Val: *big.NewInt(int64(r)), Rune: true}}) +} + +func nodflt(f float64) *Node { + return nodlit(Val{&Mpflt{Val: *big.NewFloat(f)}}) +} + +func TestCaseClauseByConstVal(t *testing.T) { + tests := []struct { + a, b *Node }{ - // Non-constants. - { - caseClause{node: Nod(OXXX, nil, nil)}, - caseClause{node: Nod(OXXX, nil, nil), isconst: true}, - +1, - }, - { - caseClause{node: Nod(OXXX, nil, nil), isconst: true}, - caseClause{node: Nod(OXXX, nil, nil)}, - -1, - }, - // Type switches - { - caseClause{node: Nod(OXXX, Nodintconst(0), nil), isconst: true}, - caseClause{node: Nod(OXXX, Nodbool(true), nil), isconst: true}, - -1, - }, - { - caseClause{node: Nod(OXXX, Nodbool(true), nil), isconst: true}, - caseClause{node: Nod(OXXX, Nodintconst(1), nil), isconst: true}, - +1, - }, - { - caseClause{node: Nod(OXXX, &Node{Type: &Type{Etype: TBOOL, Vargen: 1}}, nil), isconst: true}, - caseClause{node: Nod(OXXX, &Node{Type: &Type{Etype: TINT, Vargen: 0}}, nil), isconst: true}, - +1, - }, - { - caseClause{node: Nod(OXXX, &Node{Type: &Type{Etype: TBOOL, Vargen: 1}}, nil), isconst: true}, - caseClause{node: Nod(OXXX, &Node{Type: &Type{Etype: TINT, Vargen: 1}}, nil), isconst: true}, - -1, - }, - { - caseClause{node: Nod(OXXX, &Node{Type: &Type{Etype: TBOOL, Vargen: 0}}, nil), isconst: true}, - caseClause{node: Nod(OXXX, &Node{Type: &Type{Etype: TINT, Vargen: 1}}, nil), isconst: true}, - -1, - }, - // Constant values. // CTFLT - { - caseClause{node: Nod(OXXX, nodlit(Val{&Mpflt{Val: *big.NewFloat(0.1)}}), nil), isconst: true}, - caseClause{node: Nod(OXXX, nodlit(Val{&Mpflt{Val: *big.NewFloat(0.2)}}), nil), isconst: true}, - -1, - }, - { - caseClause{node: Nod(OXXX, nodlit(Val{&Mpflt{Val: *big.NewFloat(0.1)}}), nil), isconst: true}, - caseClause{node: Nod(OXXX, nodlit(Val{&Mpflt{Val: *big.NewFloat(0.1)}}), nil), isconst: true}, - 0, - }, - { - caseClause{node: Nod(OXXX, nodlit(Val{&Mpflt{Val: *big.NewFloat(0.2)}}), nil), isconst: true}, - caseClause{node: Nod(OXXX, nodlit(Val{&Mpflt{Val: *big.NewFloat(0.1)}}), nil), isconst: true}, - +1, - }, + {nodflt(0.1), nodflt(0.2)}, // CTINT - { - caseClause{node: Nod(OXXX, Nodintconst(0), nil), isconst: true}, - caseClause{node: Nod(OXXX, Nodintconst(1), nil), isconst: true}, - -1, - }, - { - caseClause{node: Nod(OXXX, Nodintconst(1), nil), isconst: true}, - caseClause{node: Nod(OXXX, Nodintconst(1), nil), isconst: true}, - 0, - }, - { - caseClause{node: Nod(OXXX, Nodintconst(1), nil), isconst: true}, - caseClause{node: Nod(OXXX, Nodintconst(0), nil), isconst: true}, - +1, - }, + {Nodintconst(0), Nodintconst(1)}, // CTRUNE - { - caseClause{node: Nod(OXXX, nodlit(Val{&Mpint{Val: *big.NewInt('a'), Rune: true}}), nil), isconst: true}, - caseClause{node: Nod(OXXX, nodlit(Val{&Mpint{Val: *big.NewInt('b'), Rune: true}}), nil), isconst: true}, - -1, - }, - { - caseClause{node: Nod(OXXX, nodlit(Val{&Mpint{Val: *big.NewInt('b'), Rune: true}}), nil), isconst: true}, - caseClause{node: Nod(OXXX, nodlit(Val{&Mpint{Val: *big.NewInt('b'), Rune: true}}), nil), isconst: true}, - 0, - }, - { - caseClause{node: Nod(OXXX, nodlit(Val{&Mpint{Val: *big.NewInt('b'), Rune: true}}), nil), isconst: true}, - caseClause{node: Nod(OXXX, nodlit(Val{&Mpint{Val: *big.NewInt('a'), Rune: true}}), nil), isconst: true}, - +1, - }, + {nodrune('a'), nodrune('b')}, // CTSTR - { - caseClause{node: Nod(OXXX, nodlit(Val{"ab"}), nil), isconst: true}, - caseClause{node: Nod(OXXX, nodlit(Val{"abc"}), nil), isconst: true}, - -1, - }, - { - caseClause{node: Nod(OXXX, nodlit(Val{"abc"}), nil), isconst: true}, - caseClause{node: Nod(OXXX, nodlit(Val{"xyz"}), nil), isconst: true}, - -1, - }, - { - caseClause{node: Nod(OXXX, nodlit(Val{"abc"}), nil), isconst: true}, - caseClause{node: Nod(OXXX, nodlit(Val{"abc"}), nil), isconst: true}, - 0, - }, - { - caseClause{node: Nod(OXXX, nodlit(Val{"abc"}), nil), isconst: true}, - caseClause{node: Nod(OXXX, nodlit(Val{"ab"}), nil), isconst: true}, - +1, - }, - { - caseClause{node: Nod(OXXX, nodlit(Val{"xyz"}), nil), isconst: true}, - caseClause{node: Nod(OXXX, nodlit(Val{"abc"}), nil), isconst: true}, - +1, - }, - // Everything else should compare equal. - { - caseClause{node: Nod(OXXX, nodnil(), nil), isconst: true}, - caseClause{node: Nod(OXXX, nodnil(), nil), isconst: true}, - 0, - }, + {nodlit(Val{"ab"}), nodlit(Val{"abc"})}, + {nodlit(Val{"ab"}), nodlit(Val{"xyz"})}, + {nodlit(Val{"abc"}), nodlit(Val{"xyz"})}, } - for i, d := range testdata { - got := exprcmp(d.a, d.b) - if d.want != got { - t.Errorf("%d: exprcmp(a, b) = %d; want %d", i, got, d.want) - t.Logf("\ta = caseClause{node: %#v, isconst: %v}", d.a.node, d.a.isconst) - t.Logf("\tb = caseClause{node: %#v, isconst: %v}", d.b.node, d.b.isconst) + for i, test := range tests { + a := caseClause{node: Nod(OXXX, test.a, nil)} + b := caseClause{node: Nod(OXXX, test.b, nil)} + s := caseClauseByConstVal{a, b} + if less := s.Less(0, 1); !less { + t.Errorf("%d: caseClauseByConstVal(%v, %v) = false", i, test.a, test.b) + } + if less := s.Less(1, 0); less { + t.Errorf("%d: caseClauseByConstVal(%v, %v) = true", i, test.a, test.b) } } }