-
Notifications
You must be signed in to change notification settings - Fork 17.9k
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: some problems on optimization when takes care about of arrays in struct #20022
Comments
We don't use the issue tracker for discussions. If you just want to talk about this, please raise it on the mailing list golang-dev. If there is a specific bug here that you think should be addressed, please be precise: give one test case, and show why the output is sub-optimal. Right now I find this issue too complex--I don't know what is broken and what should be fixed. I also don't care about the performance of code compiled with |
Also you should disassemble go1.8 code at least (even better: tip, the current master). Reports that point out deficiencies in the go1.7 compiler are not very useful because whoever process your report will have to re-check everything with tip just to understand if the issues you reported are still valid or not. |
@ianlancetaylor Actually, I use -B to make the assemble easy to read. And the bug(I think it's a bug) is also there when remove -B. And I hope this can be fixed(by removing useless operations). |
@ALTree Go 1.8 is the same as Go 1.7,4. I should put Go 1.8 instead of Go 1.7. But I really tried Go 1.8. This situation is always there. |
@ianlancetaylor @ALTree Changed to 1.8.1, and show the problem more clearly. |
Both of these cases (II and IV) are still happening at tip. In II, that extra instruction is a nil check. The compiler does not yet know that it can lift that nil check out of the loop. You could do something like
before the loop to lift that nil check out yourself. In IV, the extra move is an unfortunate side-effect of how we track unsafe.Pointer <-> uintptr conversions. We are careful in the compiler to keep those two types separate to keep the pointer maps for the garbage collector accurate. This means we end up with an extra MOVQconvert that isn't necessary. |
For IV (the extra move), see #21572. |
As @ianlancetaylor and @ALTree mentioned, I changed the version from 1.7.4 to 1.8.1 to show the bug clearly.
What version of Go are you using (
go version
)?What operating system and processor architecture are you using (
go env
)?What did you do?
I test several examples to test the performance of iterating an array. All of the version are compile by
go build
version I
I put the array into a struct
The assemble of function sub is
And the loop is
The line
44d5bd
is quite strange. What's the usage oftest
here?? I think it's useless, and it will waste a cycle. We care about it, because we will use this kind of function a lot.version II
We use a way to make the compiler do not what we are passing.
The assembler of function sub
The loop is
The number of operations in version II is the same as version I, but memory read times have been reduced, so this kind of version is faster.
version III
To reduce the numbers of operations in version II, I changed again.
The assemble of function sub
The loop is
The number of operations are not reduced. The reason is
44d5d2
and44d5d2
.The text was updated successfully, but these errors were encountered: