cmd/compile: inlined err check elimination opportunity

[egonelbre]

I realized that there is a if err != nil branch elimination possibility after inlined func.

With go tip 2c2980a, the following code (https://go.godbolt.org/z/sx5Gr9):

package ex

func fuse(v int) (int, error) {
	s, err := sqr(v)
	if err != nil {
		return -1, err
	}
	return s, nil
}

func sqr(v int) (int, error) {
	if v < 0 {
		return 0, errorString("xyz")
	}
	return v * v, nil
}

type errorString string

func (s errorString) Error() string { return string(s) }

This is compiled into:

        movq    "".v+8(SP), AX
        testq   AX, AX
        jge     fuse_pc65
        leaq    go.itab."".errorString,error(SB), AX
        xorl    CX, CX
        leaq    ""..stmp_0(SB), DX
fuse_pc26:
        testq   AX, AX // <--- here it should be possible to know that `AX` is not-nil
        jeq     fuse_pc51
        movq    $-1, "".~r1+16(SP)
        movq    AX, "".~r2+24(SP)
        movq    DX, "".~r2+32(SP)
        ret
fuse_pc51:
        movq    CX, "".~r1+16(SP)
        xorps   X0, X0
        movups  X0, "".~r2+24(SP)
        ret
fuse_pc65:
        imulq   AX, AX
        movq    AX, CX
        xorl    DX, DX
        xorl    AX, AX
        jmp     fuse_pc26

If I manually do the transform, the code ends up looking like:

func fuse(v int) (int, error) {
    if v < 0 {
        return -1, errorString("xyz")
    }
	return v*v, nil
}

Which, in assembly is:

        movq    "".v+8(SP), AX
        testq   AX, AX
        jge     fuse_pc65
        movq    $-1, "".~r1+16(SP)
        leaq    go.itab."".errorString,error(SB), AX
        movq    AX, "".~r2+24(SP)
        leaq    ""..stmp_0(SB), AX
        movq    AX, "".~r2+32(SP)
        ret
fuse_pc65:
        imulq   AX, AX
        movq    AX, "".~r1+16(SP)
        xorps   X0, X0
        movups  X0, "".~r2+24(SP)
        ret

I'm not sure for how many err != nil it would be possible, however, already for a few, it should improve things.

When creating the err inside the func, it's able to remove the branch:

func fuse(v int) (int, error) {
	var err error = errorString("xyz")
	if err != nil {
		return -1, err
	}
	return v * v, nil
}

Compiles into:

        movq    $-1, "".~r1+16(SP)
        leaq    go.itab."".errorString,error(SB), AX
        movq    AX, "".~r2+24(SP)
        leaq    ""..stmp_0(SB), AX
        movq    AX, "".~r2+32(SP)
        ret

[egonelbre]

Could be related to #17862

[randall77]

There's a recombination of control flow happening here that the compiler isn't currently able to disentangle.
Basically, the inlined version is doing this:

func fuse(v int) (int, error) {
	var s int
	var err error
	if v < 0 {
		s = -1
		err = errorString("xyz")
	} else {
		s = v*v
		err = nil
	}
	// recombination here
	if err != nil {
		return -1, err
	}
	return s, nil
}

This should certainly be optimizable. Try patching in https://go-review.googlesource.com/c/go/+/239457 , it may just be that optimization. But maybe not quite, because there's one pesky instruction in one of the branches that needs to be lifted first?

At the fuse pass, we have

b3: ← b1
Plain → b2 (+27)

b4: ← b1-
v16 (+29) = Mul64 <int> v7 v7 (s[int])
Plain → b2 (+30)

b2: ← b3 b4-
v32 (35) = Phi <int> v11 v16 (s[int])
v31 (32) = Phi <uintptr> v12 v20 (err.itab[uintptr])
v4 (32) = Phi <*uint8> v13 v19 (err.data[*uint8])
v18 (+32) = IsNonNil <bool> v31
If v18 → b6 b5 (32)

The Mul64 might prevent that CL from helping.
@erifan