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.

Sign up for GitHub

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

Jump to bottom

cmd/compile: struct equality code optimization #39428

Closed
randall77 opened this issue Jun 5, 2020 · 3 comments
Closed

cmd/compile: struct equality code optimization #39428

randall77 opened this issue Jun 5, 2020 · 3 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Performance
Milestone
Go1.16

Comments

@randall77
Copy link
Contributor 

type S struct {
	a, b, c string
}
type A [3]string

func f(x, y S) bool {
	return x == y
}
func g(x, y A) bool {
	return x == y
}

The array equality function looks optimal. It compares each length in turn and branches to "return false" if any don't match. If all the lengths match, then it calls memequal in a loop for each string.

The code for struct equality looks worse.

	0x0021 00033 (<autogenerated>:1)	MOVQ	"".p+48(SP), AX
	0x0026 00038 (<autogenerated>:1)	MOVQ	8(AX), CX
	0x002a 00042 (<autogenerated>:1)	MOVQ	"".q+56(SP), DX
	0x002f 00047 (<autogenerated>:1)	MOVQ	(DX), BX
	0x0032 00050 (<autogenerated>:1)	MOVQ	(AX), SI
	0x0035 00053 (<autogenerated>:1)	CMPQ	8(DX), CX
	0x0039 00057 (<autogenerated>:1)	JEQ	198
	0x003f 00063 (<autogenerated>:1)	XORL	CX, CX
	0x0041 00065 (<autogenerated>:1)	TESTB	CL, CL
	0x0043 00067 (<autogenerated>:1)	JEQ	194

	0x00c6 00198 (<autogenerated>:1)	MOVQ	SI, (SP)
	0x00ca 00202 (<autogenerated>:1)	MOVQ	BX, 8(SP)
	0x00cf 00207 (<autogenerated>:1)	MOVQ	CX, 16(SP)
	0x00d4 00212 (<autogenerated>:1)	CALL	runtime.memequal(SB)
	0x00d9 00217 (<autogenerated>:1)	MOVBLZX	24(SP), CX
	0x00de 00222 (<autogenerated>:1)	MOVQ	"".p+48(SP), AX
	0x00e3 00227 (<autogenerated>:1)	MOVQ	"".q+56(SP), DX
	0x00e8 00232 (<autogenerated>:1)	JMP	65

The length-same (calling memequal) and length-different branches recombine, requiring retesting of the result, which then skips over the remaining tests (and not all the rest, but one at a time).

@josharian
@randall77 randall77 added this to the Go1.16 milestone Jun 5, 2020
@dmitshur dmitshur added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jun 5, 2020
@dmitshur
Copy link
Contributor

dmitshur commented Jun 5, 2020

@randall77 Thanks for opening this. This issue was missing one of the Needs{Investigation,Decision,Fix} labels that triaged issues should have. I've added NeedsInvestigation, but you can pick another if an issue you're reporting is in a different state. Thanks.

@josharian
Copy link
Contributor

Thanks. I’m still AFK but I suspect (purely from the issue text) that it’d be easier/better to fix this by strengthening the shortcircuit pass more.

@gopherbot
Copy link

Change https://golang.org/cl/255317 mentions this issue: cmd/compile: optimize generated struct/array equality code

@golang golang locked and limited conversation to collaborators Oct 28, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Performance
Projects
None yet
Milestone
Go1.16
Development

No branches or pull requests
4 participants
@josharian @dmitshur @randall77 @gopherbot