1
0
mirror of https://github.com/golang/go synced 2024-11-17 13:04:54 -07:00

cmd/compile: add ellipsis rule diagnostics to rulegen

These detect opportunities to convert a rule to use an ellipsis,
and provide better error messages when something goes wrong.

This change was used to generate all the preceding changes
converting rules to use ellipses. This change is at the end of those
changes rather than the beginning in order to avoid log spam during rule
generation (say during a git bisection).

The preceding changes collectively shrink the cmd/compile binary by ~2.2%.

Part of this detection is also warning when the presence of an
unmentioned aux or auxint could cause conversion to an ellipsis
rule to change the sematics of the rule.

For example:

(Div64 x y) -> (DIV x y)

looks like a promising rule for an ellipsis. However, Div64 has an auxint,
and (on most platforms) DIV does not. An ellipsis rule would keep the
auxint intact, rather than zeroing it, which can infere with CSE.
So this change flags this rule as doing implicit zeroing;
it should be replaced by

(Div64 [a] x y) -> (DIV x y)

which makes it clear that the auxint is being zeroed.

This detection is not foolproof, but it currently has no false positives.
If false positives arise in the future, we will need to gate the output.

Change-Id: Ie21f284579e5d6e75aa304d0deb024d41ede528b
Reviewed-on: https://go-review.googlesource.com/c/go/+/217014
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
This commit is contained in:
Josh Bleecher Snyder 2020-01-23 14:33:26 -08:00
parent b8f54e57c2
commit a3fc77aa7e

View File

@ -1287,23 +1287,31 @@ func parseValue(val string, arch arch, loc string) (op opData, oparch, typ, auxi
}
// Sanity check aux, auxint.
if auxint != "" {
switch op.aux {
case "Bool", "Int8", "Int16", "Int32", "Int64", "Int128", "Float32", "Float64", "SymOff", "SymValAndOff", "TypSize":
default:
log.Fatalf("%s: op %s %s can't have auxint", loc, op.name, op.aux)
}
if auxint != "" && !opHasAuxInt(op) {
log.Fatalf("%s: op %s %s can't have auxint", loc, op.name, op.aux)
}
if aux != "" {
switch op.aux {
case "String", "Sym", "SymOff", "SymValAndOff", "Typ", "TypSize", "CCop", "ArchSpecific":
default:
log.Fatalf("%s: op %s %s can't have aux", loc, op.name, op.aux)
}
if aux != "" && !opHasAux(op) {
log.Fatalf("%s: op %s %s can't have aux", loc, op.name, op.aux)
}
return
}
func opHasAuxInt(op opData) bool {
switch op.aux {
case "Bool", "Int8", "Int16", "Int32", "Int64", "Int128", "Float32", "Float64", "SymOff", "SymValAndOff", "TypSize":
return true
}
return false
}
func opHasAux(op opData) bool {
switch op.aux {
case "String", "Sym", "SymOff", "SymValAndOff", "Typ", "TypSize", "CCop", "ArchSpecific":
return true
}
return false
}
// splitNameExpr splits s-expr arg, possibly prefixed by "name:",
// into name and the unprefixed expression.
// For example, "x:(Foo)" yields "x", "(Foo)",
@ -1533,11 +1541,20 @@ func normalizeMatch(m string, arch arch) string {
func parseEllipsisRules(rules []Rule, arch arch) (newop string, ok bool) {
if len(rules) != 1 {
for _, r := range rules {
if strings.Contains(r.rule, "...") {
log.Fatalf("%s: found ellipsis in rule, but there are other rules with the same op", r.loc)
}
}
return "", false
}
rule := rules[0]
match, cond, result := rule.parse()
if cond != "" || !isEllipsisValue(match) || !isEllipsisValue(result) {
if strings.Contains(rule.rule, "...") {
log.Fatalf("%s: found ellipsis in non-ellipsis rule", rule.loc)
}
checkEllipsisRuleCandidate(rule, arch)
return "", false
}
op, oparch, _, _, _, _ := parseValue(result, arch, rule.loc)
@ -1556,6 +1573,41 @@ func isEllipsisValue(s string) bool {
return true
}
func checkEllipsisRuleCandidate(rule Rule, arch arch) {
match, cond, result := rule.parse()
if cond != "" {
return
}
op, _, _, auxint, aux, args := parseValue(match, arch, rule.loc)
var auxint2, aux2 string
var args2 []string
var usingCopy string
if result[0] != '(' {
// Check for (Foo x) -> x, which can be converted to (Foo ...) -> (Copy ...).
args2 = []string{result}
usingCopy = " using Copy"
} else {
_, _, _, auxint2, aux2, args2 = parseValue(result, arch, rule.loc)
}
// Check that all restrictions in match are reproduced exactly in result.
if aux != aux2 || auxint != auxint2 || len(args) != len(args2) {
return
}
for i := range args {
if args[i] != args2[i] {
return
}
}
switch {
case opHasAux(op) && aux == "" && aux2 == "":
fmt.Printf("%s: rule silently zeros aux, either copy aux or explicitly zero\n", rule.loc)
case opHasAuxInt(op) && auxint == "" && auxint2 == "":
fmt.Printf("%s: rule silently zeros auxint, either copy auxint or explicitly zero\n", rule.loc)
default:
fmt.Printf("%s: possible ellipsis rule candidate%s: %q\n", rule.loc, usingCopy, rule.rule)
}
}
func opByName(arch arch, name string) opData {
name = name[2:]
for _, x := range genericOps {