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: loop condition loop-invariant expression not lifted out of the loop #47919

Closed
prattmic opened this issue Aug 23, 2021 · 1 comment

Comments

@prattmic
Copy link
Member

package main

//go:noinline
func nop() {
}

//go:noinline
func loop(n, d int) {
	for i := 0; i < n*d; i++ {
		nop()
	}
}

func main() {
	loop(32, 16)
}

https://play.golang.org/p/RxXnFIJqtcj

loop compiles to:

00000 (29) TEXT "".loop(SB), ABIInternal
00001 (29) FUNCDATA $0, gclocals·33cdeccccebe80329f1fdbee7f5874cb(SB)
00002 (29) FUNCDATA $1, gclocals·33cdeccccebe80329f1fdbee7f5874cb(SB)
00003 (29) FUNCDATA $5, "".loop.arginfo1(SB)
00004 (29) MOVQ AX, "".n(SP)
00005 (29) MOVQ BX, "".d+8(SP)
00006 (29) XORL CX, CX
00007 (30) JMP 15
00008 (30) MOVQ CX, "".i-8(SP)
00009 (+31) PCDATA $1, $0
00010 (+31) CALL "".nop(SB)
00011 (+30) MOVQ "".i-8(SP), AX
00012 (30) LEAQ 1(AX), CX
00013 (30) MOVQ "".n(SP), AX
00014 (30) MOVQ "".d+8(SP), BX
00015 (+30) IMULQ BX, AX
00016 (30) CMPQ CX, AX
00017 (30) JLT 8
00018 (33) PCDATA $1, $-1
00019 (33) RET

PCs 13-15 (IMULQ and associated loads) could be hoisted out of the loop to avoid re-computing the product on every iteration.


The original motivation of this issue is from an application I was profiling with a hot path something like:

func process(s []float64, d int) {
  for i := 0; i < len(s)/d; i++ {
    ... = asmVectorOps(s[i*d:(i+1)*d])
  }
}

The custom assembly used most of the time, but 5% of cycles still went to the division of len(s)/d on every iteration. I thought this was an optimization breaking down because it thought len(s) might change, but it turns out not to depend on slices at all.

cc @randall77 @dr2chase @mdempsky

Potentially related to or duplicate of #15808.

@prattmic prattmic added this to the Backlog milestone Aug 23, 2021
@prattmic
Copy link
Member Author

After reading #15808 more closely, I'd say this is a duplicate.

@golang golang locked and limited conversation to collaborators Aug 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants