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

Comments

Projects
None yet
6 participants
@josharian
Copy link
Contributor

commented Mar 9, 2017

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

This comment has been minimized.

Copy link
Contributor

commented Mar 9, 2017

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

This comment has been minimized.

Copy link
Member

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

This comment has been minimized.

Copy link

commented Jan 3, 2018

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

@randall77

This comment has been minimized.

Copy link
Contributor

commented Jan 3, 2018

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

@mundaym

This comment has been minimized.

Copy link
Member

commented Jan 4, 2018

@randall77 Yep, I can add support for s390x.

@gopherbot gopherbot closed this in 4b00d3f Feb 26, 2018

@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.
You can’t perform that action at this time.