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

spec: specify dead code elimination behavior #21424

Closed
gstupp opened this Issue Aug 13, 2017 · 12 comments

Comments

Projects
None yet
10 participants
@gstupp

gstupp commented Aug 13, 2017

I have been using dead code elimination as a way to circumvent the fact that Go does not support #ifdef. This generally works well but seems fragile so ,for closed source projects, it would be very helpful if there was a clear specification when code is assured to be eliminated.
Ideally the spec would include the elimination of logical expressions that can be evaluated as false during compilation ("ifdef" like patterns), as well as the elimination of parameters that are passed to empty functions (e.g. turn off logs as in log.h)

Gideon.

@polarina

This comment has been minimized.

Show comment
Hide comment
@polarina

polarina Aug 13, 2017

Contributor

You may be able to achieve the same goal with the use of dependency inversion.

type Logger interface {
	Info(message string)
}

At program start-up time, you can choose which implementation of Logger you want to use.

type ActualLogger struct {
}

func (l *ActualLogger) Info(message string) {
	fmt.Println(message)
}
type BlackholeLogger struct {
}

func (l *BlackholeLogger) Info(message string) {
	// Do nothing!
}

You can pass these dependencies explicitly to those parts of your code that output logs. You can also use a global variable:

var logger = ActualLogger{}
func doStuff() {
	logger.Info("This log entry may either get printed or not")
}
Contributor

polarina commented Aug 13, 2017

You may be able to achieve the same goal with the use of dependency inversion.

type Logger interface {
	Info(message string)
}

At program start-up time, you can choose which implementation of Logger you want to use.

type ActualLogger struct {
}

func (l *ActualLogger) Info(message string) {
	fmt.Println(message)
}
type BlackholeLogger struct {
}

func (l *BlackholeLogger) Info(message string) {
	// Do nothing!
}

You can pass these dependencies explicitly to those parts of your code that output logs. You can also use a global variable:

var logger = ActualLogger{}
func doStuff() {
	logger.Info("This log entry may either get printed or not")
}
@mvdan

This comment has been minimized.

Show comment
Hide comment
@mvdan

mvdan Aug 13, 2017

Member

I believe that what @gstupp is after is removing certain code - dead code - from binaries, since he mentioned closed source.

I believe the spec is being kept separate from the main compiler implementation on purpose. In fact, the Go team maintains two separate compilers; there's also gccgo.

But for a definite answer, you want someone like @griesemer or @ianlancetaylor.

Member

mvdan commented Aug 13, 2017

I believe that what @gstupp is after is removing certain code - dead code - from binaries, since he mentioned closed source.

I believe the spec is being kept separate from the main compiler implementation on purpose. In fact, the Go team maintains two separate compilers; there's also gccgo.

But for a definite answer, you want someone like @griesemer or @ianlancetaylor.

@mvdan mvdan changed the title from Spec'ing dead code elimination behavior to spec: specify dead code elimination behavior Aug 13, 2017

@randall77

This comment has been minimized.

Show comment
Hide comment
@randall77

randall77 Aug 13, 2017

Contributor

I don't think we want to specify dead code elimination in the spec. For one, it's not observable within the language. Second, it would preclude simple implementations of the language (e.g. gopherjs: https://github.com/gopherjs/gopherjs).

When you say it "seems fragile", what do you mean exactly?
The current gc compiler should always eliminate code surrounded by if cond { ... } if cond is a constant (in the language spec sense) that evaluates to false.

Contributor

randall77 commented Aug 13, 2017

I don't think we want to specify dead code elimination in the spec. For one, it's not observable within the language. Second, it would preclude simple implementations of the language (e.g. gopherjs: https://github.com/gopherjs/gopherjs).

When you say it "seems fragile", what do you mean exactly?
The current gc compiler should always eliminate code surrounded by if cond { ... } if cond is a constant (in the language spec sense) that evaluates to false.

@faiface

This comment has been minimized.

Show comment
Hide comment
@faiface

faiface Aug 13, 2017

I'm pretty sure this does not belong to the spec as it is not related to syntax nor semantics, but to a compiler implementation.

faiface commented Aug 13, 2017

I'm pretty sure this does not belong to the spec as it is not related to syntax nor semantics, but to a compiler implementation.

@josharian

This comment has been minimized.

Show comment
Hide comment
@josharian

josharian Aug 14, 2017

Contributor

In addition to what the others have written:

You may find build tags to be a better way to simulate #ifdef. This is (more or less) the officially supported way, and even the current state of dead code elimination is far from perfect. For example, you can't use dead code elimination to add/remove top-level declarations like methods and exported variables, but you can use build tags. See the next paragraph for another example.

Even specifying what "dead code elimination" means is complicated. For example, it is required just that there be no instructions corresponding to the eliminable code? Or should other side-effects of the presence of the code be eliminated, such as impacting whether a function can be inlined, or whether variables escape? Prior to 1.9 (which is not even out yet), dead code still impacted inlining and escape analysis. This is definitely not the sort of thing we'd want to discuss in the spec.

Note that even in 1.9, there are multiple kinds of dead code elimination, some of which will remove these side-effects and others of which will not. The earlier DCE, which currently generally works on evaluation of constant expressions, will eliminate them, and the later DCE, which has access to far more sophisticated data flow analysis, will not. But this has and will change over time, and that's good.

You may find this blog post I wrote amusing (or horrifying) reading.

Contributor

josharian commented Aug 14, 2017

In addition to what the others have written:

You may find build tags to be a better way to simulate #ifdef. This is (more or less) the officially supported way, and even the current state of dead code elimination is far from perfect. For example, you can't use dead code elimination to add/remove top-level declarations like methods and exported variables, but you can use build tags. See the next paragraph for another example.

Even specifying what "dead code elimination" means is complicated. For example, it is required just that there be no instructions corresponding to the eliminable code? Or should other side-effects of the presence of the code be eliminated, such as impacting whether a function can be inlined, or whether variables escape? Prior to 1.9 (which is not even out yet), dead code still impacted inlining and escape analysis. This is definitely not the sort of thing we'd want to discuss in the spec.

Note that even in 1.9, there are multiple kinds of dead code elimination, some of which will remove these side-effects and others of which will not. The earlier DCE, which currently generally works on evaluation of constant expressions, will eliminate them, and the later DCE, which has access to far more sophisticated data flow analysis, will not. But this has and will change over time, and that's good.

You may find this blog post I wrote amusing (or horrifying) reading.

@faiface

This comment has been minimized.

Show comment
Hide comment
@faiface

faiface Aug 14, 2017

Furthermore, dead code detection is an undecidable problem, which basically means that there are arbitrarily good solutions to it. Including a random one of them in the spec doesn't sound like a particularly good idea.

faiface commented Aug 14, 2017

Furthermore, dead code detection is an undecidable problem, which basically means that there are arbitrarily good solutions to it. Including a random one of them in the spec doesn't sound like a particularly good idea.

@mvdan

This comment has been minimized.

Show comment
Hide comment
@mvdan

mvdan Aug 14, 2017

Member

I believe build tags as suggested by Josh are the right solution here - if some source code files aren't compiled at all as per the spec, there's no way the code can find its way to the final binary. And they're more powerful and predictable than dead code elimination.

@gstupp have you tried using build tags?

Member

mvdan commented Aug 14, 2017

I believe build tags as suggested by Josh are the right solution here - if some source code files aren't compiled at all as per the spec, there's no way the code can find its way to the final binary. And they're more powerful and predictable than dead code elimination.

@gstupp have you tried using build tags?

@gstupp

This comment has been minimized.

Show comment
Hide comment
@gstupp

gstupp Aug 14, 2017

@josharian interesting blog post.

I am aware of the fact that DCE is undecidable in general and changes between implementations in practice (hence "fragile") and I do use tags when I can. But I did not find a simple solution for cases where fine-grained control is necessary. For example, to implement an internal debug log I set a constant bool according to the build tag

// +build EXTERNAL,!INTERNAL
const INTERNAL bool = false 

and then encapsulate every call to the debuglog with a conditional to make sure that in external builds the strings that are passed to the log are removed from the binary.

func main() {
...
	if INTERNAL {
		debuglog.Println("Internal IP, must not be exposed")
	}
...
	
}

This seems to be working for now, but there is no guarantee it will work in the future. Still, I don't know of any other way to do this since debug log messages are everywhere.

gstupp commented Aug 14, 2017

@josharian interesting blog post.

I am aware of the fact that DCE is undecidable in general and changes between implementations in practice (hence "fragile") and I do use tags when I can. But I did not find a simple solution for cases where fine-grained control is necessary. For example, to implement an internal debug log I set a constant bool according to the build tag

// +build EXTERNAL,!INTERNAL
const INTERNAL bool = false 

and then encapsulate every call to the debuglog with a conditional to make sure that in external builds the strings that are passed to the log are removed from the binary.

func main() {
...
	if INTERNAL {
		debuglog.Println("Internal IP, must not be exposed")
	}
...
	
}

This seems to be working for now, but there is no guarantee it will work in the future. Still, I don't know of any other way to do this since debug log messages are everywhere.

@davecheney

This comment has been minimized.

Show comment
Hide comment
@davecheney

davecheney Aug 14, 2017

Contributor
Contributor

davecheney commented Aug 14, 2017

@gstupp

This comment has been minimized.

Show comment
Hide comment
@gstupp

gstupp Aug 14, 2017

I will follow your suggestion and open bugs if and when I hit DCE issues. But because a critical feature of my code depends on an unspecified behavior of the compiler I have to continue using the minimalist mechanism I have today.

gstupp commented Aug 14, 2017

I will follow your suggestion and open bugs if and when I hit DCE issues. But because a critical feature of my code depends on an unspecified behavior of the compiler I have to continue using the minimalist mechanism I have today.

@cznic

This comment has been minimized.

Show comment
Hide comment
@cznic

cznic Aug 14, 2017

Contributor

Perhaps confirm consider maintaining separate branches of your project in the VCS.

Contributor

cznic commented Aug 14, 2017

Perhaps confirm consider maintaining separate branches of your project in the VCS.

@griesemer

This comment has been minimized.

Show comment
Hide comment
@griesemer

griesemer Aug 14, 2017

Contributor

@gstupp As others have already said, putting dead code elimination in the spec is a non-starter: it's really an implementation detail and as @randall77 pointed out, it's not observable in the language and thus doesn't belong into the language spec.

Using constant expressions (which are well defined in the spec) that evaluate to true or false and using those to conditionally "enable/disable" code via if statements is a standard way to control such behavior. The cmd/compile compiler does and will continue to support this.

Closing this as this is not an issue. If you meant this to be a proposal you can re-open it and tag it as proposal. Thanks.

Contributor

griesemer commented Aug 14, 2017

@gstupp As others have already said, putting dead code elimination in the spec is a non-starter: it's really an implementation detail and as @randall77 pointed out, it's not observable in the language and thus doesn't belong into the language spec.

Using constant expressions (which are well defined in the spec) that evaluate to true or false and using those to conditionally "enable/disable" code via if statements is a standard way to control such behavior. The cmd/compile compiler does and will continue to support this.

Closing this as this is not an issue. If you meant this to be a proposal you can re-open it and tag it as proposal. Thanks.

@griesemer griesemer closed this Aug 14, 2017

@golang golang locked and limited conversation to collaborators Aug 14, 2018

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