compiler: fix min/max on floats by using intrinsics

This fixes two edge cases for min/max:
min/max(+0.0, -0.0) = -0.0, +0.0
min/max(number, NaN) = NaN, NaN

The compare and select method does not work here.
I switched to the llvm.minimum/llvm.maximum intrinsics which match the intended behavior.

The integer min/max were also swapped over to using intrinsics.
The compare and select path is now only used by strings.

compiler/compiler.go

@@ -1716,20 +1716,66 @@ func (b *builder) createBuiltin(argTypes []types.Type, argValues []llvm.Value, c
 		return llvmLen, nil
 	case "min", "max":
 		// min and max builtins, added in Go 1.21.
-		// We can simply reuse the existing binop comparison code, which has all
+		// Find the corresponding intrinsic name.
-		// the edge cases figured out already.
+		ty := argTypes[0].Underlying().(*types.Basic)
-		tok := token.LSS
+		llvmType := b.getLLVMType(ty)
-		if callName == "max" {
+		info := ty.Info()
-			tok = token.GTR
+		var prefix, delimeter, typeName string
-		}
+		if info&types.IsInteger != 0 {
-		result := argValues[0]
+			// This is an integer value.
-		typ := argTypes[0]
+			// Use the LLVM int min/max intrinsics.
-		for _, arg := range argValues[1:] {
+			prefix = "llvm.s"
-			cmp, err := b.createBinOp(tok, typ, typ, result, arg, pos)
+			if info&types.IsUnsigned != 0 {
-			if err != nil {
+				prefix = "llvm.u"
-				return result, err
 			}
-			result = b.CreateSelect(cmp, result, arg, "")
+			delimeter = ".i"
+			typeName = strconv.Itoa(llvmType.IntTypeWidth())
+		} else {
+			switch ty.Kind() {
+			case types.String:
+				// Strings do not have an equivalent intrinsic.
+				// Implement with compares and selects.
+				tok := token.LSS
+				if callName == "max" {
+					tok = token.GTR
+				}
+				result := argValues[0]
+				typ := argTypes[0]
+				for _, arg := range argValues[1:] {
+					cmp, err := b.createBinOp(tok, typ, typ, result, arg, pos)
+					if err != nil {
+						return result, err
+					}
+					result = b.CreateSelect(cmp, result, arg, "")
+				}
+				return result, nil
+			case types.Float32:
+				typeName = "f32"
+			case types.Float64:
+				typeName = "f64"
+			default:
+				return llvm.Value{}, b.makeError(pos, "todo: min/max: unknown type")
+			}
+			// There are a few edge cases with floating point min/max:
+			// min(-0.0, +0.0) = -0.0
+			// min(NaN, number) = NaN
+			// The llvm.minimum.*/llvm.maximum.* intrinsics match this behavior.
+			// Neither Go nor LLVM defines the bit representation of resulting NaNs.
+			prefix = "llvm."
+			delimeter = "imum."
+		}
+		intrinsicName := prefix + callName + delimeter + typeName
+		// Find or create the intrinsic.
+		llvmFn := b.mod.NamedFunction(intrinsicName)
+		if llvmFn.IsNil() {
+			fnType := llvm.FunctionType(llvmType, []llvm.Type{llvmType, llvmType}, false)
+			llvmFn = llvm.AddFunction(b.mod, intrinsicName, fnType)
+		}
+		// Call the intrinsic repeatedly to merge the arguments.
+		callType := llvmFn.GlobalValueType()
+		result := argValues[0]
+		for _, arg := range argValues[1:] {
+			result = b.CreateCall(callType, llvmFn, []llvm.Value{result, arg}, "")
 		}
 		return result, nil
 	case "panic":

compiler/testdata/go1.21.ll

@@ -22,6 +22,9 @@ entry:
   ret i32 %a
 }
 
+; Function Attrs: nocallback nofree nosync nounwind speculatable willreturn memory(none)
+declare i32 @llvm.smin.i32(i32, i32) #3
+
 ; Function Attrs: nounwind
 define hidden i32 @main.min2(i32 %a, i32 %b, ptr %context) unnamed_addr #2 {
 entry:
@@ -53,6 +56,9 @@ entry:
   ret i8 %0
 }
 
+; Function Attrs: nocallback nofree nosync nounwind speculatable willreturn memory(none)
+declare i8 @llvm.umin.i8(i8, i8) #3
+
 ; Function Attrs: nounwind
 define hidden i32 @main.minUnsigned(i32 %a, i32 %b, ptr %context) unnamed_addr #2 {
 entry:
@@ -60,22 +66,29 @@ entry:
   ret i32 %0
 }
 
+; Function Attrs: nocallback nofree nosync nounwind speculatable willreturn memory(none)
+declare i32 @llvm.umin.i32(i32, i32) #3
+
 ; Function Attrs: nounwind
 define hidden float @main.minFloat32(float %a, float %b, ptr %context) unnamed_addr #2 {
 entry:
-  %0 = fcmp olt float %a, %b
+  %0 = call float @llvm.minimum.f32(float %a, float %b)
-  %1 = select i1 %0, float %a, float %b
+  ret float %0
-  ret float %1
 }
 
+; Function Attrs: nocallback nofree nosync nounwind speculatable willreturn memory(none)
+declare float @llvm.minimum.f32(float, float) #3
+
 ; Function Attrs: nounwind
 define hidden double @main.minFloat64(double %a, double %b, ptr %context) unnamed_addr #2 {
 entry:
-  %0 = fcmp olt double %a, %b
+  %0 = call double @llvm.minimum.f64(double %a, double %b)
-  %1 = select i1 %0, double %a, double %b
+  ret double %0
-  ret double %1
 }
 
+; Function Attrs: nocallback nofree nosync nounwind speculatable willreturn memory(none)
+declare double @llvm.minimum.f64(double, double) #3
+
 ; Function Attrs: nounwind
 define hidden %runtime._string @main.minString(ptr readonly %a.data, i32 %a.len, ptr readonly %b.data, i32 %b.len, ptr %context) unnamed_addr #2 {
 entry:
@@ -100,6 +113,9 @@ entry:
   ret i32 %0
 }
 
+; Function Attrs: nocallback nofree nosync nounwind speculatable willreturn memory(none)
+declare i32 @llvm.smax.i32(i32, i32) #3
+
 ; Function Attrs: nounwind
 define hidden i32 @main.maxUint(i32 %a, i32 %b, ptr %context) unnamed_addr #2 {
 entry:
@@ -107,14 +123,19 @@ entry:
   ret i32 %0
 }
 
+; Function Attrs: nocallback nofree nosync nounwind speculatable willreturn memory(none)
+declare i32 @llvm.umax.i32(i32, i32) #3
+
 ; Function Attrs: nounwind
 define hidden float @main.maxFloat32(float %a, float %b, ptr %context) unnamed_addr #2 {
 entry:
-  %0 = fcmp ogt float %a, %b
+  %0 = call float @llvm.maximum.f32(float %a, float %b)
-  %1 = select i1 %0, float %a, float %b
+  ret float %0
-  ret float %1
 }
 
+; Function Attrs: nocallback nofree nosync nounwind speculatable willreturn memory(none)
+declare float @llvm.maximum.f32(float, float) #3
+
 ; Function Attrs: nounwind
 define hidden %runtime._string @main.maxString(ptr readonly %a.data, i32 %a.len, ptr readonly %b.data, i32 %b.len, ptr %context) unnamed_addr #2 {
 entry:
@@ -139,7 +160,7 @@ entry:
 }
 
 ; Function Attrs: nocallback nofree nounwind willreturn memory(argmem: write)
-declare void @llvm.memset.p0.i32(ptr nocapture writeonly, i8, i32, i1 immarg) #3
+declare void @llvm.memset.p0.i32(ptr nocapture writeonly, i8, i32, i1 immarg) #4
 
 ; Function Attrs: nounwind
 define hidden void @main.clearZeroSizedSlice(ptr %s.data, i32 %s.len, i32 %s.cap, ptr %context) unnamed_addr #2 {
@@ -156,24 +177,9 @@ entry:
 
 declare void @runtime.hashmapClear(ptr dereferenceable_or_null(40), ptr) #1
 
-; Function Attrs: nocallback nofree nosync nounwind speculatable willreturn memory(none)
-declare i32 @llvm.smin.i32(i32, i32) #4
-
-; Function Attrs: nocallback nofree nosync nounwind speculatable willreturn memory(none)
-declare i8 @llvm.umin.i8(i8, i8) #4
-
-; Function Attrs: nocallback nofree nosync nounwind speculatable willreturn memory(none)
-declare i32 @llvm.umin.i32(i32, i32) #4
-
-; Function Attrs: nocallback nofree nosync nounwind speculatable willreturn memory(none)
-declare i32 @llvm.smax.i32(i32, i32) #4
-
-; Function Attrs: nocallback nofree nosync nounwind speculatable willreturn memory(none)
-declare i32 @llvm.umax.i32(i32, i32) #4
-
 attributes #0 = { allockind("alloc,zeroed") allocsize(0) "alloc-family"="runtime.alloc" "target-features"="+bulk-memory,+bulk-memory-opt,+call-indirect-overlong,+mutable-globals,+nontrapping-fptoint,+sign-ext,-multivalue,-reference-types" }
 attributes #1 = { "target-features"="+bulk-memory,+bulk-memory-opt,+call-indirect-overlong,+mutable-globals,+nontrapping-fptoint,+sign-ext,-multivalue,-reference-types" }
 attributes #2 = { nounwind "target-features"="+bulk-memory,+bulk-memory-opt,+call-indirect-overlong,+mutable-globals,+nontrapping-fptoint,+sign-ext,-multivalue,-reference-types" }
-attributes #3 = { nocallback nofree nounwind willreturn memory(argmem: write) }
+attributes #3 = { nocallback nofree nosync nounwind speculatable willreturn memory(none) }
-attributes #4 = { nocallback nofree nosync nounwind speculatable willreturn memory(none) }
+attributes #4 = { nocallback nofree nounwind willreturn memory(argmem: write) }
 attributes #5 = { nounwind }

testdata/go1.21.go

@@ -1,15 +1,25 @@
 package main
 
+import "math"
+
 func main() {
 	// The new min/max builtins.
+	// With int:
 	ia := 1
 	ib := 5
 	ic := -3
+	println("min/max:", min(ia, ib, ic), max(ia, ib, ic))
+	// With float:
 	fa := 1.0
 	fb := 5.0
 	fc := -3.0
-	println("min/max:", min(ia, ib, ic), max(ia, ib, ic))
 	println("min/max:", min(fa, fb, fc), max(fa, fb, fc))
+	// Float +/- 0.0:
+	pos0 := 0.0
+	neg0 := -pos0
+	println("min/max:", min(pos0, neg0), max(pos0, neg0))
+	// Float NaN:
+	println("min/max:", min(math.NaN(), 12.0), max(math.NaN(), 12.0))
 
 	// The clear builtin, for slices.
 	s := []int{1, 2, 3, 4, 5}

testdata/go1.21.txt

@@ -1,5 +1,7 @@
 min/max: -3 5
 min/max: -3.000000e+000 +5.000000e+000
+min/max: -0.000000e+000 +0.000000e+000
+min/max: NaN NaN
 cleared s[:3]: 0 0 0 4 5
 cleared map:    0
 added to cleared map:    four 1