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: remove instruction reordering by obj package #15837

Closed
randall77 opened this issue May 25, 2016 · 4 comments
Closed

cmd/compile: remove instruction reordering by obj package #15837

randall77 opened this issue May 25, 2016 · 4 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. Performance
Milestone
Go1.9Early

Comments

@randall77
Copy link
Contributor

It's confusing, especially to assembly writers, when obj package reorders their instructions from under them. It also interferes with any instruction layout done earlier in the compiler, e.g. SSA scheduling.

It may be as simple as removing the Follow pass from cmd/internal/obj/plist.go (and all the associated code for each arch).

The follow pass does the following optimization: for unconditional branches, copy a few instructions from the destination instead of jumping to the copy.

For typical loops, SSA currently generates:

loop:
   CMP ...
   JGE exit
   ..loop body..
   jmp loop
exit:

The follow pass rewrites this to:

   CMP ...
   JGE exit
loop:
   ..loop body..
   CMP ...
   JLT loop
exit:

We probably want to do this loop head peeling some other way (in SSA?).

@rsc
@randall77 randall77 added this to the Go1.8 milestone May 25, 2016
@rsc
Copy link
Contributor

rsc commented May 25, 2016

SGTM

@dr2chase
Copy link
Contributor

I agree, and do we get better register allocation if we peel the loop head pre-regalloc? Doing this in SSA also has the possibility of automatically hoisting loop-invariant expressions out through CSE, depending on how much the CMP drags with it.

@quentinmit quentinmit added the NeedsFix The path to resolution is known, but the work has not been done. label Oct 11, 2016
@rsc rsc modified the milestones: Go1.9Early, Go1.8 Oct 21, 2016
This was referenced Mar 20, 2017
Closed
Closed
@gopherbot
Copy link

CL https://golang.org/cl/38431 mentions this issue.

@randall77 randall77 mentioned this issue Mar 25, 2017
Closed
@gopherbot
Copy link

CL https://golang.org/cl/38664 mentions this issue.

gopherbot pushed a commit that referenced this issue Mar 26, 2017
@randall77
cmd/compile: remove likely bits from generated assembly
214be5b 
We don't need them any more since #15837 was fixed.

Fixes #19718

Change-Id: I13e46c62b321b2c9265f44c977b63bfb23163ca2
Reviewed-on: https://go-review.googlesource.com/38664
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
@navytux navytux mentioned this issue Mar 26, 2017
Open
@golang golang locked and limited conversation to collaborators Apr 24, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. Performance
Projects
None yet
Milestone
Go1.9Early
Development

No branches or pull requests
5 participants
@rsc @quentinmit @dr2chase @randall77 @gopherbot