cmd/compile: doesn't optimize float comparison to MINSD

[dominikh]

Go version

go version go1.24.1 linux/amd64

Output of go env in your module/workspace:

-

What did you do?

go build -gcflags=-S

func mymin(a, b float64) float64 {
	if a < b {
		return a
	} else {
		return b
	}
}

What did you see happen?

0x0000 00000 (bar.go:4)      UCOMISD X0, X1
0x0004 00004 (bar.go:4)      JLS     7
0x0006 00006 (bar.go:5)      RET
0x0007 00007 (bar.go:7)      MOVUPS  X1, X0
0x000a 00010 (bar.go:7)      RET

What did you expect to see?

I think

MINSD X1, X0
RET

but maybe

MINSD X0, X1
MOVUPS X1, X0
RET

AT&T argument order has me thoroughly confused at times.

[randall77]

We have to be very careful about NaNs and +/-0. I don't think MINSD does the same thing that your code does for some of those cases.
It can be tricky, see

go/src/cmd/compile/internal/ssa/_gen/AMD64.rules

Line 175 in a68bf75

// Floating-point min is tricky, as the hardware op isn't right for various special

[gabyhelp]

Related Issues

• cmd/compile: optimize comparisons to min/max (u)ints by checking for overflow #41664 (closed)
• gopls/internal/analysis/modernize: minmax changes semantics of floating point comparisons involving NaN #72829
• cmd/compile: detect and apply more CMOV optimizations #27780

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

[dominikh]

I forgot about negative zero. I think you're right. mymin(+0.0, -0.0) and mymin(-0.0, +0.0) both return +0.0, whereas MINSD is documented to

If the values being compared are both 0.0s (of either sign), the value in the second source operand is returned

Thanks!

[dominikh]

.. Of course the literal -0.0 doesn't do what you might expect in Go. With

zero := 0.0
println(mymin(+0.0, -zero))
println(mymin(-zero, +0.0))

we get -0.0 and +0.0, matching MINSD.

[randall77]

I guess it is possible that would work. All the non-commutative cases hurt my brain...
I guess MINSD was designed for exactly this case, at least in C semantics.

[dominikh]

This runs to completion:

-- baz.go --
package main

import (
	"fmt"
	"math"
)

func mymin(a, b float64) float64 {
	if a < b {
		return a
	} else {
		return b
	}
}

func minsd(a, b float64) float64

func main() {
	zero := 0.0
	negZero := -zero
	cases := []float64{zero, negZero, math.NaN(), 42, -42, math.Inf(-1), math.Inf(1)}

	for _, a := range cases {
		for _, b := range cases {
			if ret1, ret2 := mymin(a, b), minsd(a, b); ret1 != ret2 {
				if math.IsNaN(ret1) && math.IsNaN(ret2) {
					continue
				}
				panic(fmt.Sprintf("%v %v, %v %v", a, b, ret1, ret2))
			}
		}
	}
}
-- baz.s --
// Code generated by command: go run asm.go. DO NOT EDIT.

// func minsd(a float64, b float64) float64
// Requires: SSE2
TEXT ·minsd(SB), $0-24
	MOVSD a+0(FP), X0
	MOVSD b+8(FP), X1
	MINSD X1, X0
	MOVSD X0, ret+16(FP)
	RET

I do apologize for making us think about floats.

[randall77]

I do apologize for making us think about floats.

You shall be punished by having to write tests about floats.

[dr2chase]

Can this be closed? (This is making me think about floats and SIMD, ugh.)

[dominikh]

As far as I understand, we've concluded that the suggested optimization is (probably) safe, so I don't think this issue should be closed yet, sorry.