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: adding a constant to a memory location needs 3 instructions #15054

Open
brtzsnr opened this Issue Mar 31, 2016 · 5 comments

Comments

Projects
None yet
6 participants
@brtzsnr
Contributor

brtzsnr commented Mar 31, 2016

Please answer these questions before submitting your issue. Thanks!

  1. What version of Go are you using (go version)?
    go version devel +e775b8d Thu Mar 31 18:41:30 2016 +0000 linux/amd64
  2. What operating system and processor architecture are you using (go env)?
    linux amd64
  3. What did you do?
    Compiled the following function
func g(a *int) {
        *a += 3
}
  1. What did you expect to see?
    One single instruction
ADDQ (CX), $3
  1. What did you see instead?
    Three instructions
MOVQ (CX), AX
ADDQ $3, AX
MOVQ AX, (CX)
@brtzsnr

This comment has been minimized.

Contributor

brtzsnr commented Mar 31, 2016

@randall77 I don't know if it's easy or not to handle this.

@minux

This comment has been minimized.

Member

minux commented Mar 31, 2016

@randall77

This comment has been minimized.

Contributor

randall77 commented Mar 31, 2016

This should be easier than #14587, as we don't have to deal with flags. You'd just need to make sure there are no other uses of the load. Then it would just be a simple rewrite.
The challenge is just the bulk of it. You'd want all the possible addressing modes, plus all the ops (ADD, SUB, AND...). In addition, this is something that only x86 can do, all the other archs are load-store architectures which can't do this.
I'm leaning toward punting on this for now. When the SSA backend gets more mature we can revisit.

@rasky

This comment has been minimized.

Member

rasky commented Apr 1, 2016

We could... generate the rules :)

@bradfitz bradfitz added the Performance label Apr 7, 2016

@bradfitz bradfitz added this to the Unplanned milestone Apr 7, 2016

@gopherbot

This comment has been minimized.

gopherbot commented Oct 17, 2016

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

gopherbot pushed a commit that referenced this issue Oct 17, 2016

cmd/compile: merge loads into operations on s390x
Adds the new canMergeLoad function which can be used by rules to
decide whether a load can be merged into an operation. The function
ensures that the merge will not reorder the load relative to memory
operations (for example, stores) in such a way that the block can no
longer be scheduled.

This new function enables transformations such as:

MOVD 0(R1), R2
ADD  R2, R3

to:

ADD  0(R1), R3

The two-operand form of the following instructions can now read a
single memory operand:

 - ADD
 - ADDC
 - ADDW
 - MULLD
 - MULLW
 - SUB
 - SUBC
 - SUBE
 - SUBW
 - AND
 - ANDW
 - OR
 - ORW
 - XOR
 - XORW

Improves SHA3 performance by 6-8%.

Updates #15054.

Change-Id: Ibcb9122126cd1a26f2c01c0dfdbb42fe5e7b5b94
Reviewed-on: https://go-review.googlesource.com/29272
Run-TryBot: Michael Munday <munday@ca.ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment