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: [dev.ssa] no-opt build broken #12347

Closed
josharian opened this issue Aug 26, 2015 · 4 comments
Closed

cmd/compile: [dev.ssa] no-opt build broken #12347

josharian opened this issue Aug 26, 2015 · 4 comments
Labels
FrozenDueToAge
Milestone
Go1.6

Comments

@josharian
Copy link
Contributor

CL 13683 broke the no-opt (-gcflags="-N -l") build. The first failure log is:

http://build.golang.org/log/51a0a72aa149162a4a96a94a432c3160310c41fb

I haven't looked into it at all.
@josharian josharian added this to the Go1.6 milestone Aug 26, 2015
@gopherbot
Copy link
Contributor

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

gopherbot pushed a commit that referenced this issue Aug 28, 2015
@randall77
[dev.ssa] cmd/compile/internal/ssa: add arg-dominating check, fix phi…

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
GPG key ID: B5690EEEBB952194
Verified
Learn about vigilant mode
f8093b8 
…elim

Add a check to make sure value arguments dominate the value.

Phi elim output used to fail this test.  When eliminating
redundant phis, phi elim was using one of the args and not
the ultimate source.  For example:

          b1: x = ...
          -> b2 b3

b2: y = Copy x        b3: z = Copy x
-> b4                 -> b4

          b4: w = phi y z

Phi elim eliminates w, but it used to replace w with (Copy y).
That's bad as b2 does not dominate b4.  Instead we should
replace w with (Copy x).

Fixes #12347

Change-Id: I9f340cdabcda8e2e90359fb4f9250877b1fffe98
Reviewed-on: https://go-review.googlesource.com/13986
Reviewed-by: David Chase <drchase@google.com>
@randall77
Copy link
Contributor

Ok, that fix was somewhat of a red herring. What's going on is this:

b1 -> b2
b2 -> b3
b3 -> b2

All of which are on the dead side of a constant branch. b2 uses a value x from b1. When we detect that b1 is dead during machine-independent optimization, we mark b1 as dead and remove all its values, including x. But b2 & b3 form a loop, so they are not removed during this phase (they will be during deadcode, but that is a separate pass). So the user of x is still around, and check complains that it has a missing arg.

@randall77
Copy link
Contributor 

func f_ssa(x int, p *int) {
    if false {
        y := x + 5
        for {
            *p = y
        }
    }
}

@gopherbot
Copy link
Contributor

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

gopherbot pushed a commit that referenced this issue Aug 29, 2015
@randall77
[dev.ssa] cmd/compile/internal/ssa: handle dead code a different way
186cf1b 
Instead of trying to delete dead code as soon as we find it, just
mark it as dead using a PlainAndDead block kind.  The deadcode pass
will do the real removal.

This way is somewhat more efficient because we don't need to mess
with successor and predecessor lists of all the dead blocks.

Fixes #12347

Change-Id: Ia42d6b5f9cdb3215a51737b3eb117c00bd439b13
Reviewed-on: https://go-review.googlesource.com/14033
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
@ianlancetaylor ianlancetaylor changed the title [dev.ssa] cmd/compile: no-opt build broken cmd/compile: [dev.ssa] no-opt build broken Sep 3, 2015
@golang golang locked and limited conversation to collaborators Sep 4, 2016
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
Go1.6
Development

No branches or pull requests
3 participants
@josharian @randall77 @gopherbot