Go: cmd/compile: transform environment variable GOSSAFUNC into the option that is passed to compiler

[ysmolsky]

Right now GOSSAFUNC is a magical environment variable that allows to peek under the hood of SSA generation. It is an indispensable tool, and we need to think about converting it from magical hidden environment variable into command line option supplied to go tool compile.

I propose this option: -ssafunc=<function>

My plan is read this option to the global variable and use that in place of GOSSAFUNC.
And in inlining phase I want to record to which functions calls were inlined from *n if ssafunc==n.funcname(). It is to display the sources of all inlined functions in ssa.html.

Also the comment by @rsc in src/cmd/go/internal/work/exec.go motivated me:

    // TODO(rsc): Convince compiler team not to add more magic environment variables,
    // or perhaps restrict the environment variables passed to subprocesses.
    magic := []string{
        "GOCLOBBERDEADHASH",
        "GOSSAFUNC",
        "GO_SSA_PHI_LOC_CUTOFF",
        "GOSSAHASH",
    }

CC: @josharian @randall77

[josharian]

SGTM.

I'd suggest -ssadump instead, though. GOSSAFUNC was introduced while developing the SSA backend. It was used to enable SSA for a particular func. Now that it is mostly about dumping information, may as well update its name.

I'd suggest also converting GOSSAHASH to -hash while we're at it. (A hash could be useful in any part of the compiler, not just SSA.)

And maybe GO_SSA_PHI_LOC_CUTOFF to something like -phicutoff. Or maybe better just delete it. I don't see it being used much in the future, and if it is needed, it will be on a transient basis, and can be locally hacked in again.

That leaves only GOCLOBBERDEADHASH. I don't have an opinion about that one. Maybe Keith does.

cc also @dr2chase @cherrymui

[cherrymui]

I personally feel that environment variable is easier to write in the command line, because it can be always at the beginning of the line, instead of somewhere in the middle. That said, I'm open to this change.

magical hidden

I'm also not sure this is a bad thing. It is supposed to be used for people who work on the compiler, not general use.

[dr2chase]

I haven't used GO_SSA_PHI_LOC_CUTOFF in ages, if it vanished I would not notice.

-ssadump would work fine for me, though it is much easier to put it at the beginning of the command line.
If we could lose the spew to standard out, that might be nice, but it would also be nice to be able to get at the tree IR too (the tree input to SSA appears at the beginning of the spew). Note that we also have the ability to dump text-formatted SSA for a given function/phase already -- it was done this way for debugging giant machine-generated files where the full dump is so large that editing is painful.

Is this "-gcflags -ssadump" or -gcflags -d=-ssadump?.

Would it we be acceptable to also print an informational message about where the file was put, since it will be some package's source directory? That can sometimes be a little tricky.

GOSSAHASH I haven't used lately, but when I do use it, I tend to use a harness that sets the environment variable(s), and the harness can cope with multi-point errors though those are rare.

So if it were up to me:

• lose GO_SSA_PHI_LOC_CUTOFF
• improve/replace GOSSAFUNC
• leave GOSSAHASH alone (for now)

[randall77]

I would leave CLOBBERDEADHASH. When using this feature, it's often not clear which compiler invocation caused it (especially the calls of the compiler that originate from test files).

[josharian]

Is this "-gcflags -ssadump" or -gcflags -d=-ssadump?.

IIUC, the proposal is "-gcflags -ssadump".

If we could lose the spew to standard out, that might be nice, but it would also be nice to be able to get at the tree IR too

I believe that Yury plans to add the Node AST dump as second column in ssa.html. So: source code, then AST, then initial SSA, then passes. At that point, I agree: We could eliminate all stdout spew, which would be really nice.

Would it we be acceptable to also print an informational message about where the file was put, since it will be some package's source directory?

Yes, that'd be great.

[ysmolsky]

I'm cleaning the std output of GOSSAFUNC. I have removed IR after eash phase. I leave the Node AST as it is because we don't dump it into ssa.html. What should I do with progs being printed to std? We have it in ssa.html already. Am I okay to remove progs output from stdout?

[ysmolsky]

Trying to convert GOSSAFUNC I came to this required mumbo-jumbo:
go build -gcflags=all=-ssadump=genssa cmd/compile

Compare it to GOSSAFUNC=genssa go build cmd/compile

I think that having GOSSAFUNC is not so bad after all. I am not sure if that would be an improvement. Thoughts?

[cherrymui]

I'm cleaning the std output of GOSSAFUNC. I have removed IR after eash phase.

Personally I would't want the IR dump being removed. When I debug large functions, with thousands of Values, searching/grepping in a text file is much easier and faster than searching in browser. (Of course you could argue that I could grep the html instead...)

[ysmolsky]

Personally I would't want the IR dump being removed.

I don't want to break anyones habits. I guess this is a no-go? Is there any sense to split dump into stdout and ssa.html as separate options? I guess not much.

I can finish the "sources of all inlined functions" (#25904) with the old environment variable.

[gopherbot]

Change https://golang.org/cl/126604 mentions this issue: cmd/compile: cache the value of environment variable GOSSAFUNC

[gopherbot]

Change https://golang.org/cl/126603 mentions this issue: cmd/compile: clean the output of GOSSAFUNC

[dr2chase]

I should add, the existing dump option (which I don't recommend for general use) is

-gcflags="all=ssa/_phase_/dump=_funcname_"

with special values "build" and "all" for _phase_. This writes one file per phase+function, with sequence numbers added to simplify ordering and also in case of collisions. It's not very nice,
but it gets the debugging out even when the input is huge (and buffering ssa.html then becomes a problem).

[ysmolsky]

Those changes above do not convert GOSSAFUNC into -ssadump yet, I did them to allow the inlined functions sources feature. I am hesitant to kill the GOSSAFUNC. I found that it is much easier to use it rather than the gcflags parameter.

About writing into stdout when GOSSAFUNC is provided. Should we perhaps introduce some modifier to printout into stdout? I really don't want to have stdout when I want to have ssa.html. I want to enable to see stuff in stdout occasionally. Maybe GOSSAFUNC=Foo+ could tell the compiler to dump to ssa.html and stdout, while GOSSAFUNC=Foo would dump only to ssa.html?

[cherrymui]

I want to enable to see stuff in stdout occasionally. Maybe GOSSAFUNC=Foo+ could tell the compiler to dump to ssa.html and stdout, while GOSSAFUNC=Foo would dump only to ssa.html?

SGTM.

[ysmolsky]

Cool. I have incorporated this behaviour into the https://go-review.googlesource.com/c/126603/

[ysmolsky]

Closing it since the solution turned out to be acceptable for the most. Let me repeat the way it was resolved:

GOSSAFUNC=Foo+ tells the compiler to dump to ssa.html and stdout, while GOSSAFUNC=Foo will dump only to ssa.html

Feel free to reopen if you want any additions.