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

Closed
brtzsnr opened this issue Mar 31, 2016 · 6 comments
Closed

cmd/compile: adding a constant to a memory location needs 3 instructions #15054

brtzsnr opened this issue Mar 31, 2016 · 6 comments
Labels
FrozenDueToAge Performance
Milestone
Unplanned

Comments

@brtzsnr
Copy link
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
Copy link
Contributor Author

brtzsnr commented Mar 31, 2016

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

@minux
Copy link
Member

minux commented Mar 31, 2016 via email

@randall77
Copy link
Contributor

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
Copy link
Member

rasky commented Apr 1, 2016

We could... generate the rules :)

@bradfitz bradfitz added this to the Unplanned milestone Apr 7, 2016
@gopherbot
Copy link
Contributor

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

gopherbot pushed a commit that referenced this issue Oct 17, 2016
@mundaym
cmd/compile: merge loads into operations on s390x
1cfb5c3 
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>
@randall77
Copy link
Contributor

This is fixed.

@golang golang locked and limited conversation to collaborators Apr 1, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge Performance
Projects
None yet
Milestone
Unplanned
Development

No branches or pull requests
6 participants
@bradfitz @brtzsnr @minux @rasky @randall77 @gopherbot