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: wrong modulo code generation #12411

Closed
brtzsnr opened this issue Aug 31, 2015 · 12 comments
Closed

cmd/compile: wrong modulo code generation #12411

brtzsnr opened this issue Aug 31, 2015 · 12 comments
Labels
FrozenDueToAge
Milestone
Go1.6

Comments

@brtzsnr
Copy link
Contributor

brtzsnr commented Aug 31, 2015

The following code behaves differently on {dev.ssa, gccgo} vs {go1.4, go1.5, master}. The expected result is to print nothing and exit with exit code 0.

package main

import "fmt"
import "os"

func main() {
        if got := f1_ssa(4); got != 0 {
                fmt.Printf("f1_ssa(4) = %v, wanted 0\n", got)
                os.Exit(1)
        }
}
func f1_ssa(a4 int) int {
        switch {
        } // prevent inlining
        return 2*a4%3%(2%(a4<<2^a4%3))
}

dev.ssa works go version devel +3b7f0c9 Mon Aug 31 03:07:43 2015 +0000 linux/amd64
master broken go version devel +019297a Mon Aug 31 04:42:04 2015 +0000 linux/amd64
go1.4 broken go version go1.4.1 linux/amd64
go1.5 broken go version go1.5 linux/amd64

This is related to modulo. Works correctly on playground http://play.golang.org/p/rSBXghm4TM (Go 1.5 linux/amd64p32)
@bradfitz
Copy link
Contributor

Is your bug about Go 1.4, 1.5, or the SSA branch? Your bug report mentions all of them and is not entirely clear.

(edit: OP updated the original post several times now. My comment no longer seems to make sense.)

@brtzsnr
Copy link
Contributor Author

brtzsnr commented Aug 31, 2015

Updated the bug to make it obvious.

@randall77
Copy link
Contributor

I believe SSA is the correct one here, as 0 is the correct answer. Both 1.4 and 1.5 are wrong.

Notice that there is no way to get a negative number out of this expression if a4 is a small positive integer.

@brtzsnr
Copy link
Contributor Author

brtzsnr commented Aug 31, 2015

Right. Do you know which version runs on playground?

@randall77
Copy link
Contributor

1.5. Just run the following on the playground:

fmt.Println(runtime.Version())

@randall77 randall77 changed the title [dev/ssa] cmd/compile/internal/gc: wrong modulo code generation cmd/compile/internal/gc: wrong modulo code generation Aug 31, 2015
@randall77 randall77 added this to the Go1.6 milestone Aug 31, 2015
@ianlancetaylor
Copy link
Contributor

With gccgo I get this from go run:

f1_ssa(4) = 0, wanted -1
exit status 1

@brtzsnr
Copy link
Contributor Author

brtzsnr commented Aug 31, 2015

It's also broken on master.

% go version
go version devel +019297a Mon Aug 31 04:42:04 2015 +0000 linux/amd64
% go run failed-913821796.go

Since master, go1.4 go1.5 and master are broken I don't quiet understand why playground is correct.

@bradfitz
Copy link
Contributor

Playground is Go 1.5 linux/amd64p32 (http://play.golang.org/p/5VtpB8FPuo)

@brtzsnr
Copy link
Contributor Author

brtzsnr commented Aug 31, 2015

I updated the test to reflect that go1.4 / go1.5 / master fail.

@bradfitz
Copy link
Contributor

I'd prefer if you stop modifying old comments. It makes following discussions harder.

@ianlancetaylor ianlancetaylor changed the title cmd/compile/internal/gc: wrong modulo code generation cmd/compile: wrong modulo code generation Sep 3, 2015
@rsc rsc mentioned this issue Nov 4, 2015
Closed
@rsc
Copy link
Contributor

rsc commented Dec 14, 2015

I have a fix for this. It will probably be just in Go 1.6, not backported to existing releases. It's been there since Go 1 so I think it's safe to leave for another few months.

@gopherbot
Copy link

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

@rsc rsc closed this as completed in 03c8164 Dec 16, 2015
@golang golang locked and limited conversation to collaborators Dec 29, 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
6 participants
@bradfitz @brtzsnr @rsc @ianlancetaylor @randall77 @gopherbot