Skip to content
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: more boolean optimizations #17905

Closed
randall77 opened this issue Nov 13, 2016 · 4 comments
Closed

cmd/compile: more boolean optimizations #17905

randall77 opened this issue Nov 13, 2016 · 4 comments
Labels
FrozenDueToAge Performance Suggested Issues that may be good for new contributors looking for work to do.
Milestone

Comments

@randall77
Copy link
Contributor

func f(a *int, b *int) bool {
	if (a == nil) != (b == nil) {
		return false
	}
	return true
}

Compiles to:

	0x0000 00000 (tmp1.go:4)	MOVQ	"".a+8(FP), AX
	0x0005 00005 (tmp1.go:4)	TESTQ	AX, AX
	0x0008 00008 (tmp1.go:4)	SETNE	AL
	0x000b 00011 (tmp1.go:4)	XORL	$1, AX
	0x000e 00014 (tmp1.go:4)	MOVQ	"".b+16(FP), CX
	0x0013 00019 (tmp1.go:4)	TESTQ	CX, CX
	0x0016 00022 (tmp1.go:4)	SETNE	CL
	0x0019 00025 (tmp1.go:4)	XORL	$1, CX
	0x001c 00028 (tmp1.go:4)	CMPB	AL, CL
	0x001e 00030 (tmp1.go:4)	JEQ	38

SETNE/XORL can be replaced with SETEQ.
It might also be worthwhile to rewrite NeqB to XOR instead of SETNE/CMPB. Not sure it would help here, but it may in other places.

@randall77 randall77 added this to the Go1.9 milestone Nov 13, 2016
@mvdan
Copy link
Member

mvdan commented Nov 13, 2016

Is there a reason why the code above is in the form if x { return false; }; return true? You could just write return (a == nil) == (b == nil). Or was that just to prove the point?

@randall77
Copy link
Contributor Author

No reason, it was just simplified from an example where I saw the suboptimal code.

@josharian josharian added the Suggested Issues that may be good for new contributors looking for work to do. label Feb 28, 2017
@josharian
Copy link
Contributor

It might also be worthwhile to rewrite NeqB to XOR instead of SETNE/CMPB.

In this code, that causes a regression, because the result of the XOR then gets TESTed, whereas the SETNE flags can be re-used.

The rest is coming in a CL shortly.

@gopherbot
Copy link

CL https://golang.org/cl/42491 mentions this issue.

@golang golang locked and limited conversation to collaborators May 9, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge Performance Suggested Issues that may be good for new contributors looking for work to do.
Projects
None yet
Development

No branches or pull requests

4 participants