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: unnecessary slice literal zeroing #45573

Open
icholy opened this issue Apr 14, 2021 · 3 comments
Open

cmd/compile: unnecessary slice literal zeroing #45573

icholy opened this issue Apr 14, 2021 · 3 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsFix The path to resolution is known, but the work has not been done. Performance
Milestone

Comments

@icholy
Copy link

icholy commented Apr 14, 2021

Keyless slice literals zero the backing array and then immediately overwrite it. https://godbolt.org/z/5bx9TPo7n

What operating system and processor architecture are you using?

linux amd64

What did you do?

package main

func main() {
    a := []int{1, 2, 3, 4}
    println(a)
}

What did you expect to see?

movq    $1, ""..autotmp_2+24(SP)
movq    $2, ""..autotmp_2+32(SP)
movq    $3, ""..autotmp_2+40(SP)
movq    $4, ""..autotmp_2+48(SP)

What did you see instead?

leaq    ""..autotmp_2+24(SP), AX
xorps   X15, X15
movups  X15, (AX)
leaq    ""..autotmp_2+40(SP), CX
xorps   X15, X15
movups  X15, (CX)
movq    $1, ""..autotmp_2+24(SP)
movq    $2, ""..autotmp_2+32(SP)
movq    $3, ""..autotmp_2+40(SP)
movq    $4, ""..autotmp_2+48(SP)
@randall77
Copy link
Contributor

randall77 commented Apr 14, 2021

Yes, the deadstore pass needs to be upgraded to handle cases like this. It's been on the back burner for a while.

I think there are 2 things that are needed to make this work:

  1. Instead of just recording the size of shadowed memory at each pointer, record an offset+size. Then we can fold OffPtr arguments into the analysis easily.
  2. Handle LocalAddr ops somehow - these are currently treated as reads and mess up the analysis. The code conservatively assumes taking the address makes the zeroed temp visible, when in fact that address is only used for storing.

I think 1 is easy. 2 might be a bit harder.

@randall77 randall77 added this to the Unplanned milestone Apr 14, 2021
@randall77 randall77 added the NeedsFix The path to resolution is known, but the work has not been done. label Apr 14, 2021
@icholy
Copy link
Author

icholy commented Apr 18, 2021

My initial though was to handle this at the ir/walk stages. Once in SSA form, the deadstore pass would need to re-discover the fact that the zeroed memory is shadowed.

@networkimprov
Copy link

networkimprov commented May 15, 2021

cc @josharian

@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Jul 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsFix The path to resolution is known, but the work has not been done. Performance
Projects
None yet
Development

No branches or pull requests

4 participants