Strange behaviour with loops [1.15 backport]

[gopherbot]

@randall77 requested issue #45175 to be considered for backport to the next 1.15 minor release.

Here's a simpler reproducer:

package main

//go:noinline
func f(c bool) int {
	b := true
	x := 0
	y := 1
	for b {
		b = false
		y = x
		x = 2
		if c {
			return 3
		}
	}
	return y
}

func main() {
	println(f(false))
}

This reproducer incorrectly prints 2.
It should print 0. And it does, with -gcflags=-d=ssa/short_circuit/off.
The problem is that during short circuit, we don't take into account that the arguments to a phi are from the previous iteration, not the current iteration. That confusion leads to assigning y the current iteration's value of x, not the previous iteration's value of x.

The bug looks like it started in 1.15.

In shortcircuit.go, we need to be a bit more choosy in replaceUses to not replace uses inside a phi, as we're replacing the current iteration's values, not those which are from the previous iteration. At least, something like that; it may not be quite so simple.

@josharian

@gopherbot please open backport issues.

[gopherbot]

Change https://golang.org/cl/304529 mentions this issue: [release-branch.go1.15] cmd/compile: disable shortcircuit optimization for intertwined phi values

[randall77]

This bug is rare but causes incorrect code to be generated for loops.

[dmitshur]

Approving per discussion in a release meeting. This backport applies to both 1.16 (#45192) and 1.15 (this issue).

[gopherbot]

Closed by merging f17b659 to release-branch.go1.15.