math: consider removing Dim assembly (and possibly Max/Min too)

[mundaym]

I've just sent CL 64194 that makes dim (the generic implementation of Dim) simple enough to be easily inlineable. I've deleted the s390x assembly since I don't think it is worth having any more. I could have made it better but I think inlining the code would be more of a win. If this CL is accepted dim will look something like this:

func dim(x, y float64) float64 {
	x -= y
	switch {
	case x > 0:
		return x
	case x <= 0:
		return 0
	}
	return NaN()
}

It would be really nice if we could delete the assembly for the other platforms so that we can get rid of the assembly stubs and Dim itself can be made inlineable.

Min and Max could also be reworked to make them inlineable (and, theoretically, faster in the usual case), using code like this:

func Max(x, y float64) float64 {
        switch {
        case y > x:
                return y
        case x > y:
                return x
        case x == y:
                // return +0 in preference to -0
                return Float64frombits(Float64bits(x) & Float64bits(y))
        }
        if inf := Float64frombits(uvinf); x == inf || y == inf {
                return inf
        }
        return Float64frombits(uvnan)
}

Note that we can't use the IsInf, Inf and NaN functions because they increase the complexity of the code the compiler sees and push max over the inlining threshold.

It is less clear cut that this is better than the assembly on all platforms (arm64 seems to have an FMAX instruction for example) and my benchmarking results were more mixed, so I haven't made this change.

[mundaym]

/cc @TocarIP @cherrymui

[mundaym]

Also, if we don't care about the bit pattern used for NaN (which we probably don't) then we could just do:

func Dim(x, y float64) float64 {
        x = x - y
        if x <= 0 {
                return 0
        }
        return x
}

[gopherbot]

Change https://golang.org/cl/64194 mentions this issue: math: optimize dim and remove s390x assembly implementation

[TocarIP]

Removing asm version of Dim everywhere, to make it faster/inlinable is great. We will need to add it to cmd/compile/internal/gc/inl_test.go to avoid regressions in the future. I don't like different Nan bit-patterns
it makes code less readable, It's better to tweak inlining heuristics.

[laboger]

In your Max example above, I don't understand why there are functions for NaN() and Inf() when they are compile time constants. Based on what you are saying, if those could be represented as constants instead of function calls that should allow more functions that contain them to be inlined because they will have lower cost?

[mundaym]

@laboger Yes, if we could specify NaN and ±∞ as constants then it would be relatively easy to reduce the cost of Min and Max enough to make them inlineable.

If we had mid-stack inlining then another approach we could take would be to push NaN handling into a helper function to reduce the inlining cost (though we'd probably need to use go:noinline to get the right effect):

func Max(x, y float64) float64 {
    switch {
    case y > x:
        return y
    case x > y:
        return x
    case x == y:
        // return +0 in preference to -0
        return Float64frombits(Float64bits(x) & Float64bits(y))
    }
    // x or y is NaN (rare)
    return maxNaN(x, y)
}

// maxNaN returns +Inf if x or y is +Inf, otherwise it returns NaN.
//go:noinline
func maxNaN(x, y float64) float64 {
        if IsInf(x, 1) || IsInf(y, 1) {
                return Inf()
        }
        return NaN()
}

[gopherbot]

Change https://golang.org/cl/80695 mentions this issue: math: remove asm version of Dim

[ianlancetaylor]

As far as I can see, this is fixed.