Skip to content

cmd/compile: loop invariant code motion #64815

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
Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

cmd/compile: loop invariant code motion #64815

wants to merge 7 commits into from
+3,358 −156

Conversation

y1yang0
Copy link
Contributor

@y1yang0 y1yang0 commented Dec 20, 2023

The implementation consists of {Loop Invariant Code Motion, Loop Rotation, Type-based Alias Analysis, Loop Closed SSA Form}

Testing: attached IR tests, codegen, go test, ssacheck, golang/benchmarks, go-http-routing-benchmark

Co-authored-by: [email protected]
Co-authored-by: [email protected]
Co-authored-by: [email protected]
Co-authored-by: [email protected]

Updates #63670

@y1yang0
cmd/compile: loop invariant code motion
07403a6 
The implementation consists of {Loop Invariant Code Motion, Loop Rotation,
Type-based Alias Analysis, Loop Closed SSA Form}

Testing: attached IR tests, codegen, go test, ssacheck,
golang/benchmarks, go-http-routing-benchmark

Co-authored-by: [email protected]
Co-authored-by: [email protected]
Co-authored-by: [email protected]
Co-authored-by: [email protected]

Updates golang#63670
@gopherbot
Copy link
Contributor

This PR (HEAD: 07403a6) has been imported to Gerrit for code review.

Please visit Gerrit at https://go-review.googlesource.com/c/go/+/551381.

Important tips:

  • Don't comment on this PR. All discussion takes place in Gerrit.
  • You need a Gmail or other Google account to log in to Gerrit.
  • To change your code in response to feedback:
    • Push a new commit to the branch used by your GitHub PR.
    • A new "patch set" will then appear in Gerrit.
    • Respond to each comment by marking as Done in Gerrit if implemented as suggested. You can alternatively write a reply.
    • Critical: you must click the blue Reply button near the top to publish your Gerrit responses.
    • Multiple commits in the PR will be squashed by GerritBot.
  • The title and description of the GitHub PR are used to construct the final commit message.
    • Edit these as needed via the GitHub web interface (not via Gerrit or git).
    • You should word wrap the PR description at ~76 characters unless you need longer lines (e.g., for tables or URLs).
  • See the Sending a change via GitHub and Reviews sections of the Contribution Guide as well as the FAQ for details.

@gopherbot
Copy link
Contributor

Message from Gopher Robot:

Patch Set 1:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/551381.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from [email protected]:

Patch Set 1:

(6 comments)

Please don’t reply on this GitHub thread. Visit golang.org/cl/551381.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from qiulaidongfeng:

Patch Set 1: Run-TryBot+1

Please don’t reply on this GitHub thread. Visit golang.org/cl/551381.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gopher Robot:

Patch Set 1:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/551381.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gopher Robot:

Patch Set 1:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/551381.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gopher Robot:

Patch Set 1: TryBot-Result-1

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/551381.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from qiulaidongfeng:

Patch Set 1: -Run-TryBot

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/551381.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Keith Randall:

Patch Set 1: Hold+1

(4 comments)

Please don’t reply on this GitHub thread. Visit golang.org/cl/551381.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Yi Yang:

Patch Set 1:

(5 comments)

Please don’t reply on this GitHub thread. Visit golang.org/cl/551381.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Keith Randall:

Patch Set 1:

(4 comments)

Please don’t reply on this GitHub thread. Visit golang.org/cl/551381.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Yi Yang:

Patch Set 1:

(5 comments)

Please don’t reply on this GitHub thread. Visit golang.org/cl/551381.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Keith Randall:

Patch Set 1:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/551381.
After addressing review feedback, remember to publish your drafts!

@Oguidan
Copy link

Oguidan commented Dec 21, 2023

@y1yang0 please can you explain me what your changes is about

@y1yang0
Copy link
Contributor Author

y1yang0 commented Dec 21, 2023

@y1yang0 please can you explain me what your changes is about

Hi @Oguidan , you can see related issue, thanks.

@gopherbot
Copy link
Contributor

Message from Lynn Boger:

Patch Set 1:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/551381.
After addressing review feedback, remember to publish your drafts!

@y1yang0
fix attached IR test
b4126dc
@y1yang0
remove TBAA as it's not true in Go
7189ef6
@y1yang0
place loop backedge after all body blocks
cc41b16
@y1yang0
fix codegen/issue58166.go
f3ec1ea 
codegen/issue58166.go fails, it expected loop increment to generate
incq, but instead, it ended up generating leaq+mov.

During register allocation, v114 couldn't be allocated to the same
register r8 as v149, due to the fact that there were two uses of v114
(v136 and v146) right after v149. As a result, unexpected instructions
were produced.

b26:
v149 = ADDQconst <int> [1] v114
v135 = MULSD <float64> v87 v123
v136 = ADDSDloadidx8 <float64> v135 v50 v114 v160 ;;use of v114
v146 = MOVSDstoreidx8 <mem> v50 v114 v136 v160 ;; use of v114
v91 = CMPQ <flags> v15 v149

After loop rotation, the schedule pass calculated scores for b26,
resulting in the following order:

Before schedule:
b26:
v135 (12) = MULSD <float64> v87 v123
v136 (12) = ADDSDloadidx8 <float64> v135 v50 v114 v160
v146 (12) = MOVSDstoreidx8 <mem> v50 v114 v136 v160
v149 (+11) = ADDQconst <int> [1] v114
v91 (+11) = CMPQ <flags> v15 v149

After schedule:
b26:
v149 = ADDQconst <int> [1] v114
v135 = MULSD <float64> v87 v123
v136 = ADDSDloadidx8 <float64> v135 v50 v114 v160
v146 = MOVSDstoreidx8 <mem> v50 v114 v136 v160
v91 = CMPQ <flags> v15 v149

An optimization was made as per CL 463751, which prioritizes
values that are used within their live blocks. Since v149 is used by v91
in the same block, it has a higher priority.

The proposed fix is that if a value is used by a control value, then it
should have the lowest priority. This would allow v146 and v91 to be
scheduled closely together. After this scheduling, there would be no use
of v114 after v146, thus allowing v146 and v114 to be allocated to the
same register, ultimately generating an incq instruction.
@y1yang0
fix test/nilptr3.go
08992fd 
func f4(x *[10]int) {
  ...
  _ = x[9] // ERROR "generated nil check"
	for {
		if x[9] != 0 { // ERROR "removed nil check"
			break
		}
	}
}

Loop rotation duplicates conditional test Values into loop gaurd block:

...
loopGuard:
v6 (+159) = NilCheck <*[10]int> v5 v1
v15 (159) = OffPtr <*int> [72] v6
v9 (159) = Load <int> v15 v1
v7 (159) = Neq64 <bool> v9 v13
If v7 → b5 loopHeader

loopHeader:
v8 (+159) = NilCheck <*[10]int> v5 v1
v11 (159) = OffPtr <*int> [72] v8
v12 (159) = Load <int> v11 v1
v14 (159) = Neq64 <bool> v12 v13
Plain → b3

So we need to add a loop nilcheckelim pass after all other loop opts to
remove the v6 NilCheck in the loop guard, but that's not the whole
story.

The code _ = x[9] is expected to generate a nilcheck, buttighten pass
will move the LoweredNilCheck from the loop entry b1 to the loop guard.
At that point, there happens to be a CMPQconstload in the loop guard
that meets the conditions for late nilcheckelim, resulting in the
elimination of the LoweredNilCheck as well. Therefore, we also need to
modify the test to no longer expect the generated code to contain a
nilcheck.
@y1yang0
fix test/writebarrier.go
30e0fea
@gopherbot
Copy link
Contributor

This PR (HEAD: 30e0fea) has been imported to Gerrit for code review.

Please visit Gerrit at https://go-review.googlesource.com/c/go/+/551381.

Important tips:

  • Don't comment on this PR. All discussion takes place in Gerrit.
  • You need a Gmail or other Google account to log in to Gerrit.
  • To change your code in response to feedback:
    • Push a new commit to the branch used by your GitHub PR.
    • A new "patch set" will then appear in Gerrit.
    • Respond to each comment by marking as Done in Gerrit if implemented as suggested. You can alternatively write a reply.
    • Critical: you must click the blue Reply button near the top to publish your Gerrit responses.
    • Multiple commits in the PR will be squashed by GerritBot.
  • The title and description of the GitHub PR are used to construct the final commit message.
    • Edit these as needed via the GitHub web interface (not via Gerrit or git).
    • You should word wrap the PR description at ~76 characters unless you need longer lines (e.g., for tables or URLs).
  • See the Sending a change via GitHub and Reviews sections of the Contribution Guide as well as the FAQ for details.

@gopherbot
Copy link
Contributor

Message from Yi Yang:

Patch Set 1:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/551381.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from [email protected]:

Patch Set 2:

(4 comments)

Please don’t reply on this GitHub thread. Visit golang.org/cl/551381.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Yi Yang:

Patch Set 2:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/551381.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from David Chase:

Patch Set 2:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/551381.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Yi Yang:

Patch Set 2:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/551381.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Shengwei Zhao:

Patch Set 2:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/551381.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Shengwei Zhao:

Patch Set 2:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/551381.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Shengwei Zhao:

Patch Set 2:

(5 comments)

Please don’t reply on this GitHub thread. Visit golang.org/cl/551381.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Shengwei Zhao:

Patch Set 2:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/551381.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Jorropo:

Patch Set 2:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/551381.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Shengwei Zhao:

Patch Set 2:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/551381.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Yi Yang:

Patch Set 2:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/551381.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Yi Yang:

Patch Set 2:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/551381.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from David Chase:

Patch Set 2:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/551381.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from David Chase:

Patch Set 2:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/551381.
After addressing review feedback, remember to publish your drafts!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@y1yang0 @gopherbot @Oguidan