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: support named values on RHS of rewrite rules #38621

Closed
josharian opened this issue Apr 23, 2020 · 4 comments
Closed

cmd/compile: support named values on RHS of rewrite rules #38621

josharian opened this issue Apr 23, 2020 · 4 comments

Comments

@josharian
Copy link
Contributor

@josharian josharian commented Apr 23, 2020

I'd like to change the amd64 splitload rules to use (TESTQ x x) instead of (CMPQconst [0] x) when possible. However, there's no way to write (TESTQ x x) on the RHS of a rewrite rule unless x is already present on the LHS. You can spell out x twice, but then it generates two distinct values. In normal rewrite rules, we rely on a subsequent CSE to clean that up. But no CSE runs after splitload. Plus that is inefficient. I'd like to be able to write something like:

(CMP(Q|L|W|B)constload {sym} [vo] ptr mem) && valOnly(vo) == 0 -> (TEST(Q|L|W|B) x:(MOV(Q|L|W|B)load {sym} [offOnly(vo)] ptr mem) x)

Comments?

cc @mvdan @randall77

@mvdan
Copy link
Member

@mvdan mvdan commented Apr 23, 2020

How does this compare to #37423? Both seem to want to add syntax for similar reasons.

@josharian
Copy link
Contributor Author

@josharian josharian commented Apr 23, 2020

#37423 contemplates calculating reusable Go values with Go expressions. This contemplates generating reusable ssa.Values with s-exprs.

@randall77
Copy link
Contributor

@randall77 randall77 commented Apr 23, 2020

That syntax seems fine to me. We already do it on the LHS.

@andybons andybons added this to the Unplanned milestone Apr 23, 2020
@gopherbot
Copy link

@gopherbot gopherbot commented Apr 23, 2020

Change https://golang.org/cl/229701 mentions this issue: cmd/compile: allow named values on RHS of rewrite rules

@gopherbot gopherbot closed this in d9d88dd Apr 23, 2020
xujianhai666 added a commit to xujianhai666/go-1 that referenced this issue May 21, 2020
Fixes golang#38621

Change-Id: Idbffdcc70903290dc58e5abb4867718bd5449fe1
Reviewed-on: https://go-review.googlesource.com/c/go/+/229701
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.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
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.