New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd/compile: add tool for understanding/debugging SSA rules #19013

Open
josharian opened this Issue Feb 9, 2017 · 11 comments

Comments

Projects
None yet
6 participants
@josharian
Contributor

josharian commented Feb 9, 2017

As discussed in CL 36329, debugging SSA rules can be tedious and time-consuming. CL 36646 adds some rudimentary tools to help. This issue describes a pie-in-the-sky tool. Maybe we'll settle on something in between. :)

ssa.html appropriately treats rule-based passes (opt, dec, lower) as a single phase, but they actually perform a very long series of transformations, and it can be hard to see how the input relates to the output. In a typical rule pass, many many rules get applied, so putting them all side-by-side is not feasible.

But imagine some html dedicated to a single rule-based pass. Two columns, before and after, similar to existing ssa.html columns. A header at the top shows a single rule being applied, and if it does not apply, why it does not apply. Before column shows input to that rule, after shows output. Use arrow keys to step forward/back to watch rules get applied (or not) one at a time.

This would be unusable without a way to filter--there are too many rules to view them all one at a time. Filtering should consist of marking either rules and/or values of interest. Any rule marked as interesting always gets shown, whether it applies or not. Any rule that alters a value of interest gets shown. All other rules get compressed into a single "n rules applied" rule.

So a typical debugging experience might be:

  • Mark rules as interesting in the rules file somehow.
  • Provide a list of interesting values, from observing previous compilations of the same code. (Where/how? envvar? file? Specifying values of interest dynamically in UI might be too hard, for data volume and sanity reasons.)
  • Compile, generating html artifact.
  • Poke around at the rules/values you care about.
  • Glorious comprehension / profit.

cc @rasky @randall77 @minux

@josharian josharian added this to the Unplanned milestone Feb 9, 2017

@dr2chase

This comment has been minimized.

Contributor

dr2chase commented Feb 9, 2017

An alternative might be to enhance the existing rule-based system. Something that took costs into account would allow the larger rule to fire before it was invalidated by sub-rules. For example, iburg. I've worked with these in the past, they're nice. In some cases you can replace compile-time dynamic programming with precompiled static tables, but those are less flexible and also lead to a certain amount of brain-hurt in their implementation.

@josharian

This comment has been minimized.

Contributor

josharian commented Feb 9, 2017

@dr2chase that sounds helpful, but unrelated to improved debugging tools for rules. If anything, it'd make the need for good debugging even higher.

@dr2chase

This comment has been minimized.

Contributor

dr2chase commented Feb 9, 2017

I didn't have that problem the two times I've used such a code generator. Normally they "just work", and an annoying class of problems (they ones that bit you in that CL) vanishes.

@minux

This comment has been minimized.

Member

minux commented Feb 10, 2017

@typeless

This comment has been minimized.

typeless commented Feb 10, 2017

I am imagining that the transformation history of the SSA graphs could perhaps be modeled as something like the organization of Git objects , and making each SSA rule act like the "author" of the commits. Then users can have a git log -L equivalent or a more flexible query language to look up the desired information. Just leave me alone if this sounds stupid.

@josharian

This comment has been minimized.

Contributor

josharian commented May 10, 2017

Something that took costs into account would allow the larger rule to fire before it was invalidated by sub-rules.

@dr2chase I really wanted this while working on CL 43157.

Quasilyte added a commit to Quasilyte/go-contributing-ru that referenced this issue Apr 1, 2018

tasks: add several new tracked tasks
New tasks include:
golang/go#19675 cmd/vet: report uses of -0 in float32/64 context
golang/go#19683 cmd/compile: eliminate usages of global lineno
golang/go#19670 x/tools/go/ssa: make opaqueType less annoying to use
golang/go#19636 encoding/base64: decoding is slow
golang/go#23471 x/perf/cmd/benchstat: tips or quickstart for newcomers
golang/go#19577 test: errorcheck support for intraline errors
golang/go#19490 cmd/vet: reduce the amount of false positives for -shadow mode
golang/go#19042 cmd/internal/obj: optimize wrapper method prologue for branch prediction
golang/go#19013 cmd/compile: add tool for understanding/debugging SSA rules
@ysmolsky

This comment has been minimized.

Member

ysmolsky commented Sep 18, 2018

Is this still wanted by anyone other than @josharian ? :)

@agnivade

This comment has been minimized.

Member

agnivade commented Oct 9, 2018

ping @josharian

@ysmolsky

This comment has been minimized.

Member

ysmolsky commented Oct 9, 2018

also ping @minux @dr2chase

@josharian

This comment has been minimized.

Contributor

josharian commented Oct 11, 2018

I still think this would be helpful (at least unless/until we have a more sophisticated rule system, such as described by @dr2chase above). The problem is that I'm still not entirely clear on what output would be most helpful.

Here's a new (simpler) idea that mimics what I sometimes do during debugging:

Add a flag to the rule generator (like -log) that accepts a file:line. For the rule on that file line, generate code to spew debug output. E.g. rewrite:

	// match: (Add64 (Const64 [c]) (Const64 [d]))
	// cond:
	// result: (Const64 [c+d])
	for {
		_ = v.Args[1]
		v_0 := v.Args[0]
		if v_0.Op != OpConst64 {
			break
		}
		c := v_0.AuxInt
		v_1 := v.Args[1]
		if v_1.Op != OpConst64 {
			break
		}
		d := v_1.AuxInt
		v.reset(OpConst64)
		v.AuxInt = c + d
		return true
	}

into something like this:

	// match: (Add64 (Const64 [c]) (Const64 [d]))
	// cond:
	// result: (Const64 [c+d])
	for {
		fmt.Println("check ", v.LongString())
		_ = v.Args[1]
		v_0 := v.Args[0]
		fmt.Println("\tv_0=", v_0.LongString())
		if v_0.Op != OpConst64 {
			fmt.Println("\tv_0.Op != OpConst64")
			break
		}
		c := v_0.AuxInt
		fmt.Println("\tc=", c.LongString())
		v_1 := v.Args[1]
		fmt.Println("\tv_1=", v_1.LongString())
		if v_1.Op != OpConst64 {
			fmt.Println("\tv_1.Op != OpConst64")
			break
		}
		d := v_1.AuxInt
		fmt.Println("\td=", v_1.AuxInt)
		v.reset(OpConst64)
		v.AuxInt = c + d
		fmt.Println("\tv.AuxInt=", c+d)
		fmt.Println("\tMATCH")
		return true
	}

I think in practice it'll just take a bit of experimenting and iterating to find out what is helpful.

@dr2chase

This comment has been minimized.

Contributor

dr2chase commented Oct 12, 2018

I think this is a plausible approach -- I looked into what it would take to add rule system w/ all the predicates we have, and it was more than I wanted to deal with right now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment