Skip to content

cmd/compile: Could make better use of "bimm64" on ARM64 #19857

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
Closed
dr2chase opened this issue Apr 5, 2017 · 5 comments
Closed

cmd/compile: Could make better use of "bimm64" on ARM64 #19857

dr2chase opened this issue Apr 5, 2017 · 5 comments
Labels
FrozenDueToAge

Comments

@dr2chase
Copy link
Contributor

dr2chase commented Apr 5, 2017

Please answer these questions before submitting your issue. Thanks!

What version of Go are you using (go version)?

1.9devel

What operating system and processor architecture are you using (go env)?

ARM64

What did you do?

Compiled x & ((1<<63)-1)

We currently convert this to a
(LSR 1 (LSL 1 x))
because that is more efficient than loading the immediate.

However, 64-bit AND apparently takes a "bimm64" operand which can contain some not entirely well specified runs of 0 and 1, that happens to include ((1<<63)-1). This came from an investigation into whether the problematic idiom described in #19809 might occur in code compiled from C or C++. I discovered that it does not, and that the operand can be expressed compactly as an immediate to logical operations.

I don't yet know if the Go assembler handles bimm64; the bug is filed so as not to forget this.
@cherrymui
Copy link
Member

Generic rule rewrites certain AND to shifts

// Rewrite AND of consts as shifts if possible, slightly faster for 64 bit operands
// leading zeros can be shifted left, then right
(And64 <t> (Const64 [y]) x) && nlz(y) + nto(y) == 64 && nto(y) >= 32
  -> (Rsh64Ux64 (Lsh64x64 <t> x (Const64 <t> [nlz(y)])) (Const64 <t> [nlz(y)]))

and we didn't undo this in ARM64 rules. Should we?

@dr2chase
Copy link
Contributor Author

dr2chase commented Apr 6, 2017

We could, it would be easy, just augment the (new) rule that converts (LSR k(LSL k x)) into (ROR k (LSL k x)) into one that uses the AND. I think no need to check the usage of the LSL because if it's > 1 that means the instruction count remains the same.

@cherrymui
Copy link
Member

Yeah, I did this. It also reveals a (performance) bug in the assembler backend that some constants did not get encoded into instruction although it could. CLs are coming.

@gopherbot
Copy link
Contributor

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

@gopherbot
Copy link
Contributor

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

gopherbot pushed a commit that referenced this issue Apr 6, 2017
@cherrymui
cmd/internal/obj/arm64: fix encoding of AND MBCON
0bae9b0 
When a constant is both MOVCON (can fit into a MOV instruction)
and BITCON (can fit into a logical instruction), the assembler
chooses to use the MOVCON encoding, which is actually longer for
logical instructions. We add MBCON rules explicitly to make sure
it uses the BITCON encoding.

Updates #19857.

Change-Id: Ib9881be363cbc491ac2a0792b36b87e74eff34a8
Reviewed-on: https://go-review.googlesource.com/39652
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
@golang golang locked and limited conversation to collaborators Apr 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@dr2chase @gopherbot @cherrymui