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: use TEST/CMP with memory operands on amd64 #19485

Closed
josharian opened this issue Mar 9, 2017 · 5 comments
Closed

cmd/compile: use TEST/CMP with memory operands on amd64 #19485

josharian opened this issue Mar 9, 2017 · 5 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. Performance release-blocker
Milestone

Comments

@josharian
Copy link
Contributor

Instrumentation after lowering suggests that a significant percentage of values on amd64 are doing a TEST or CMP on memory.

Here are some common patterns. First column is % of all values measured (during make.bash). Second column is number of values. Next column is top-level op. All following columns are ops of arguments. So 2.36% of values were a TESTL of the result of a memory load.

  2.36% 69443 VAL	TESTL	MOVLload	MOVLload
  2.20% 64610 VAL	TESTQ	MOVQload	MOVQload
  1.15% 33720 VAL	CMPQconst	MOVQload
  1.05% 30884 VAL	CMPBconst	MOVBload
  0.98% 28781 VAL	CMPLconst	MOVLload
  0.87% 25475 VAL	TESTB	MOVBload	MOVBload

CMP and TEST can accept a memory arg instead of a register.

Those collectively account for >8% of values, which makes it seem potentially worth investigating.

However, this can't be done using rewrite rules, because it could lead to multiple live memory states. It could potentially be done using a peep pass, which is Keith's favorite suggestion--see #15300. I don't have any other plans, but I wanted to file this in case anyone else saw a good way.

This is a generalization of #15245.

@josharian josharian added this to the Unplanned milestone Mar 9, 2017
@ianlancetaylor
Copy link
Contributor

This is where instruction selection runs into register allocation. In principle BURG instruction selection can handle this but I don't know whether it has ever been used in a practical compiler.

@mundaym
Copy link
Member

mundaym commented Mar 10, 2017

If a mechanism is ever introduced for this then it could also be used for s390x (which can also do comparisons with memory operands).

@gopherbot
Copy link

Change https://golang.org/cl/86035 mentions this issue: cmd/compile: implement comparisons directly with memory

@randall77
Copy link
Contributor

@mundaym : I just posted a CL to fix this for amd64. After it is in can you add s390x support?

@bradfitz bradfitz modified the milestones: Unplanned, Go1.11 Jan 4, 2018
@bradfitz bradfitz added release-blocker NeedsFix The path to resolution is known, but the work has not been done. labels Jan 4, 2018
@mundaym
Copy link
Member

mundaym commented Jan 4, 2018

@randall77 Yep, I can add support for s390x.

@golang golang locked and limited conversation to collaborators Feb 26, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. Performance release-blocker
Projects
None yet
Development

No branches or pull requests

6 participants