go:noinline directive

[Keith Randall]

We're thinking about adding a go:noinline directive to help with testing.  Currently, if you don't want a function inlined, you add a switch{} or a defer or other not-obviously-a-nop construct to the function.  We'd like to introduce a new directive called go:noinline to mark such functions, as the old way depends on a consistent definition of nop-obviousness (which we probably don't want to commit to).

We particularly need this feature on the SSA branch because if a function is inlined, the code contained in that function might switch from being SSA-compiled to old-compiler-compiled.  Without some sort of noinline mark the SSA-specific tests might not be testing the SSA backend at all.

It is possible that once everything is running under SSA this directive will be less useful, but I think it will be generically useful going forward.  Thus I'd like to discuss this now in anticipation of it being merged into mainline sometime soon.

https://go-review.googlesource.com/13911 if you're interested.

[Rob Pike]

Worth filing a proposal?

-rob

--
You received this message because you are subscribed to the Google Groups "golang-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Keith Randall]

Sure, proposal posted.

[minux]

My counter proposal is that we settle on a pattern to disable inlining and document it.

For example, I think this pattern works regardless of compiler version:

var alwaysfalse bool

func shouldNotBeInlined() {
    if alwaysfalse {
        shouldNotBeInlined()
    }
}

Because the compiler cannot determine that the bool is always false, this function is self-recursive and cannot be inlined even if recursive inlining (-l=3) is enabled (if we disregard partial function inlining). The pattern is safe until we have whole program optimization that can discover that nothing changes the value of alwaysfalse, but I imagine it's quite hard to do while maintaining the current compile speed benefit. Even if we do have such optimization in the future, detect this pattern explicitly should be easy.

It does have negligible runtime penalty though, but you're already sacrificing performance when trying to disable inlining.

[Russ Cox]

What's the benefit over //go:noinline? It seems the same, just spelled more verbosely.

Russ

[minux]

On Aug 25, 2015 2:50 PM, "Russ Cox" <r...@golang.org> wrote:
> What's the benefit over //go:noinline? It seems the same, just spelled more verbosely.
>

The benefit seems to be that it doesn't require one more directive and also it's already supported by current compilers.

Adding a new directive means the code will behave differently in compilers that doesn't support the directive (most importantly, older versions.)

[David Chase]

There do exist compilers for other languages that do the work to notice that alwaysfalse is never assigned and is thus indeed alwaysfalse, and it is generally true that when compilers do this they make people happy.  I am a huge fan of saying what you mean, and if the language lacks a way to say what needs to be said, add it.

By-the-way, even lacking the knowledge that alwaysfalse is always false, that code could handily be optimized into

func shouldNotBeInlined() {
    if alwaysfalse {
        for {}
    }
}

You'll note that the code can now be inlined because it contains no calls, and stack storage consumption has been infinitely optimized.  Three cheers for the optimizer, right?  It made your code better, isn't that what you wanted?

On Tue, Aug 25, 2015 at 2:44 PM, minux <mi...@golang.org> wrote:

--

[minux]

On Tue, Aug 25, 2015 at 2:59 PM, David Chase <drc...@google.com> wrote:

There do exist compilers for other languages that do the work to notice that alwaysfalse is never assigned and is thus indeed alwaysfalse, and it is generally true that when compilers do this they make people happy.  I am a huge fan of saying what you mean, and if the language lacks a way to say what needs to be said, add it.

By-the-way, even lacking the knowledge that alwaysfalse is always false, that code could handily be optimized into

func shouldNotBeInlined() {
    if alwaysfalse {
        for {}
    }
}

You'll note that the code can now be inlined because it contains no calls, and stack storage consumption has been infinitely optimized.  Three cheers for the optimizer, right?  It made your code better, isn't that what you wanted?

This optimization can only be applied if the there is no code before the if

or if the compiler supports tail-call optimization (none of the current go

compilers do).

I normally place the self recursive code at the end of the function to inhibit

inlining (actually, for the current gc compiler and functions returning nothing,

I don't use the if, I just place the self recursive call after an explicit return

statement, and it works well (because inlining decision is determined before

DCE for the current gc compiler))

To suppress the no-store optimization, we can export the alwaysfalse variable,

forcing the compiler to do whole program analysis to discover that no code

ever changes it to true.

I think a new directive carries significant burden than a non-ideal workaround

like this. And once we establish an official way to do it, I think the intent will

be just as clear as having a new directive.

And please note that the pattern I suggested is not the only way to achieve

the result, and we can use the many alternatives. The proposal is just about

using an existing supported code pattern instead of introducing a new

directive.

[Paul Borman]

From a readability point of view:

//go:noinline

is pretty clear.

var alwaysfalse bool

func somethingFancy(a, b string, c int) string, error{

    if alwaysfalse {

        return somethingFancy(a, b, c)

    }

    ....

}

this is cryptic and seems that any sane person, unfamiliar with it, would simply remove the clause.  You could always add a comment:

var alwaysfalse bool

func somethingFancy(a, b string, c int) string, error{

    if alwaysfalse {

        // this prevents inlining, do not remove

        return somethingFancy(a, b, c)

    }

    ....

}

but where does that get us?  Go is about saying what you mean and meaning what you say, not depending on side effects or artifacts.

    -Paul

--

[David Chase]

Don't forget that the proposed use case is for compiler testing, so one reason to favor the directive over polluting the control flow is that sometimes you want to feed the compiler code that lacks polluted control flow -- and if I had the time to work on it, I would also be working on improving inlining in ways that might still allow the inlining of your improved anti-inliner.

Look at all the various complaints about "hey, Go is slower at X than language Y".  Think about what it means to make code run 1% more efficiently across a large company's datacenters.  Over time, if Go succeeds, there will be large incentives to do floats-most-boats optimizations, and these subtext-encodings-of-intent will just look like what-were-we-thinking? technical debt when that time comes.

[Robert Griesemer]

I am concerned that people will want this feature and start relying on it; i.e., it becomes a language mechanism.

Since this is explicitly for compiler testing, can we simply not document it (widely)?

(e.g., go/types understands a couple more predeclared functions, just for testing. Those functions are not accessible outside the test harness.)

- gri

On Tue, Aug 25, 2015 at 12:42 PM, 'David Chase' via golang-dev <golan...@googlegroups.com> wrote:

Don't forget that the proposed use case is for compiler testing, so one reason to favor the directive over polluting the control flow is that sometimes you want to feed the compiler code that lacks polluted control flow -- and if I had the time to work on it, I would also be working on improving inlining in ways that might still allow the inlining of your improved anti-inliner.

Look at all the various complaints about "hey, Go is slower at X than language Y".  Think about what it means to make code run 1% more efficiently across a large company's datacenters.  Over time, if Go succeeds, there will be large incentives to do floats-most-boats optimizations, and these subtext-encodings-of-intent will just look like what-were-we-thinking? technical debt when that time comes.

[Russ Cox]

On Tue, Aug 25, 2015 at 3:54 PM, Robert Griesemer <g...@golang.org> wrote:

I am concerned that people will want this feature and start relying on it; i.e., it becomes a language mechanism.

Since this is explicitly for compiler testing, can we simply not document it (widely)?

It should be documented in 'go doc cmd/compile' with the others. It's not much different from the others. It's explicitly okay for any toolchain to ignore them.

Russ

[Rob Pike]

All these comments belong in the proposal's issue, not here.

[minux]

Ok, how about add a runtime/debug func DoNotInline() that technically is a no-op, but if called, will inhibit inlining of the enclosing function.

For now, we just need to implement that in assembly. Future improved compiler might need to detect it.

It is as clear as //go:noinline, but doesn't require changes to the compiler.