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: constants of same underlying type are not merged #15043

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

cmd/compile: constants of same underlying type are not merged #15043

brtzsnr opened this issue Mar 31, 2016 · 5 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 +d8f1f8d Wed Mar 30 22:27:13 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 program

package main

type foo int
type bar int
type spam int
type eggs int

func f(a []int) {
        _ = a[foo(3)]
        _ = a[bar(3)]
        _ = a[spam(3)]
        _ = a[eggs(3)]
}

func main() {
}
  1. What did you expect to see?
    One bounds check.
  2. What did you see instead?
    4 bounds checks. The constants foo(3), bar(3), spam(3), eggs(3) are not merged by the zcse pass nor by the constants cache. However, most rewrite rules ignore the type and only use op.

@josharian @tzneal
@josharian
Copy link
Contributor

Yep. We use the frontend's notion of the equality, which takes into account named types, etc. I think there's an item in the ssa todo file to use a looser notion of equality; good to have it as an issue too.

cc @mdempsky

@randall77
Copy link
Contributor

The particular example in this bug will be fixed by https://go-review.googlesource.com/c/21008
The more general problem of using fuzzy type equality still stands, however.

@tzneal
Copy link
Member

tzneal commented Apr 1, 2016

I might be mis-remembering, but I thought that @dr2chase had implemented a structural type equality, but didn't commit it. I think it would be fairly easy and safe to modify zcse to handle cases like this without much effort.

@gopherbot
Copy link

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

@bradfitz bradfitz added this to the Unplanned milestone Apr 9, 2016
mk0x9 pushed a commit to mk0x9/go that referenced this issue Apr 17, 2016
@josharian
cmd/compile/internal/ssa: use Compare instead of Equal
2563b6f 
They have different semantics.

Equal is stricter and is designed for the front-end.
Compare is looser and cheaper and is designed for the back-end.
To avoid possible regression, remove Equal from ssa.Type.

Updates golang#15043

Change-Id: Ie23ce75ff6b4d01b7982e0a89e6f81b5d099d8d6
Reviewed-on: https://go-review.googlesource.com/21483
Reviewed-by: David Chase <drchase@google.com>
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
@randall77
Copy link
Contributor

This looks fixed now.

@golang golang locked and limited conversation to collaborators Aug 30, 2017
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 @tzneal @josharian @randall77 @gopherbot