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

[dev.ssa] cmd/compile/internal/gc: wrong code generation #12143

Closed
brtzsnr opened this issue Aug 13, 2015 · 2 comments
Closed

[dev.ssa] cmd/compile/internal/gc: wrong code generation #12143

brtzsnr opened this issue Aug 13, 2015 · 2 comments

Comments

@brtzsnr
Copy link
Contributor

brtzsnr commented Aug 13, 2015

package main
import "fmt"
func main() {
fmt.Println(f1_ssa())
}
func f1_ssa() int8 {
switch {} // prevent inlining
v4 := uint(21) // uint
v7 := v4 & v4 * v4 >> 2 - 3 + v4 << 1 ^ v4 >> 2 + v4 * v4 + v4 >> uint32(0) * 3 | v4 & v4 << 1 - 2 ^ v4 | v4 >> (3 + 1 << 2 * 2 ^ 3 >> uint32(0) * 3 << (uint32(0) + uint32(0)) - 1 & 2) // uint
v8 := uint32(0) >> ((v7 & v4 * v4) ^ v7 * v4 * v7)
v10 := 3 << v8 | 1 * 3 + 3 >> 2 * (3) - int8(3) - (int8(3) + int8(3) >> v4) // int8
return 3 & v10 + int8(3) >> v8 - int8(3) << 2 << 1 - v10 ^ int8(3) * int8(3) - 3 | int8(3) * v10 & int8(3) & v10 << 1 >> 12
}

go1.4: -29
master: -29
dev.ssa: -30

This is a bug in dev.ssa at tip. I'll try to minimize this later if I have time.

@randall77
Copy link
Contributor

This appears to be the result of wrong flags processing. The scheduled code does:

v88 = CMPLconst <flags> [8] v48
v86 = SBBLcarrymask <int8> v88
... do things that clobber flags...
v87 = SBBLcarrymask <uint32> v88

v88 is not spilled and restored properly. There are 3 issues:

  1. Regalloc doesn't know that the intervening instructions clobber flags.
  2. We can't spill/restore flags.
  3. The two SBBLs aren't CSEd because their types are different.

1 is about to be fixed with the new regalloc.
2 is to be done, but no timeline yet.
3 Josh was thinking about. Maybe remove types from (most?) values, they aren't used in many places.

With just 1 we'll at least get a "can't be regalloced" error instead of silently generating wrong code.

@randall77 randall77 self-assigned this Aug 13, 2015
@randall77
Copy link
Contributor

Closed via https://go-review.googlesource.com/#/c/13622/
1 is done, so we get "spill of flags not implemented yet" and fallback to the old compiler.

@golang golang locked and limited conversation to collaborators Aug 22, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants