Skip to content

[stdlib] Improve performance of heterogeneous binary integer == and < #63034

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jan 18, 2023
Merged

[stdlib] Improve performance of heterogeneous binary integer == and < #63034

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d6ac4e3
[stdlib] Improve performance of generic binary integer `==` and `<`
xwu Jan 14, 2023
f671e50
[stdlib] Further optimize heterogeneous binary integer comparison
xwu Jan 15, 2023
52075f0
[stdlib] Add IRGen test for heterogeneous integer comparison.
xwu Jan 16, 2023
e19bb87
[IRGen] Amend large temporary allocation codegen test to be hopefully…
xwu Jan 16, 2023
df6a56e
[stdlib] Make a stylistic change per reviewer feedback.
xwu Jan 18, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
95 changes: 37 additions & 58 deletions stdlib/public/core/Integers.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1658,39 +1658,26 @@ extension BinaryInteger {
public static func == <
Other: BinaryInteger
>(lhs: Self, rhs: Other) -> Bool {
let lhsNegative = Self.isSigned && lhs < (0 as Self)
let rhsNegative = Other.isSigned && rhs < (0 as Other)

if lhsNegative != rhsNegative { return false }

// Here we know the values are of the same sign.
//
// There are a few possible scenarios from here:
//
// 1. Both values are negative
// - If one value is strictly wider than the other, then it is safe to
// convert to the wider type.
// - If the values are of the same width, it does not matter which type we
// choose to convert to as the values are already negative, and thus
// include the sign bit if two's complement representation already.
// 2. Both values are non-negative
// - If one value is strictly wider than the other, then it is safe to
// convert to the wider type.
// - If the values are of the same width, than signedness matters, as not
// unsigned types are 'wider' in a sense they don't need to 'waste' the
// sign bit. Therefore it is safe to convert to the unsigned type.

if lhs.bitWidth < rhs.bitWidth {
return Other(truncatingIfNeeded: lhs) == rhs
}
if lhs.bitWidth > rhs.bitWidth {
return lhs == Self(truncatingIfNeeded: rhs)
// Use bit pattern conversion to widen the comparand with smaller bit width.
if Self.isSigned == Other.isSigned {
return lhs.bitWidth >= rhs.bitWidth ?
lhs == Self(truncatingIfNeeded: rhs) :
Other(truncatingIfNeeded: lhs) == rhs
}

if Self.isSigned {
return Other(truncatingIfNeeded: lhs) == rhs
// If `Self` is signed but `Other` is unsigned, then we have to
// be a little more careful about widening, since:
// (1) a fixed-width signed type can't represent the largest values of
// a fixed-width unsigned type of equal bit width; and
// (2) an unsigned type (obviously) can't represent a negative value.
if Self.isSigned {
return lhs.bitWidth > rhs.bitWidth ? // (1)
lhs == Self(truncatingIfNeeded: rhs) :
(lhs >= (0 as Self) && Other(truncatingIfNeeded: lhs) == rhs) // (2)
}
return lhs == Self(truncatingIfNeeded: rhs)
// Analogous reasoning applies if `Other` is signed but `Self` is not.
return lhs.bitWidth < rhs.bitWidth ?
Other(truncatingIfNeeded: lhs) == rhs :
(rhs >= (0 as Other) && lhs == Self(truncatingIfNeeded: rhs))
}

/// Returns a Boolean value indicating whether the two given values are not
Expand Down Expand Up @@ -1730,34 +1717,26 @@ extension BinaryInteger {
/// - rhs: Another integer to compare.
@_transparent
public static func < <Other: BinaryInteger>(lhs: Self, rhs: Other) -> Bool {
let lhsNegative = Self.isSigned && lhs < (0 as Self)
let rhsNegative = Other.isSigned && rhs < (0 as Other)
if lhsNegative != rhsNegative { return lhsNegative }

if lhs == (0 as Self) && rhs == (0 as Other) { return false }

// if we get here, lhs and rhs have the same sign. If they're negative,
// then Self and Other are both signed types, and one of them can represent
// values of the other type. Otherwise, lhs and rhs are positive, and one
// of Self, Other may be signed and the other unsigned.

let rhsAsSelf = Self(truncatingIfNeeded: rhs)
let rhsAsSelfNegative = rhsAsSelf < (0 as Self)


// Can we round-trip rhs through Other?
if Other(truncatingIfNeeded: rhsAsSelf) == rhs &&
// This additional check covers the `Int8.max < (128 as UInt8)` case.
// Since the types are of the same width, init(truncatingIfNeeded:)
// will result in a simple bitcast, so that rhsAsSelf would be -128, and
// `lhs < rhsAsSelf` will return false.
// We basically guard against that bitcast by requiring rhs and rhsAsSelf
// to be the same sign.
rhsNegative == rhsAsSelfNegative {
return lhs < rhsAsSelf
// Use bit pattern conversion to widen the comparand with smaller bit width.
if Self.isSigned == Other.isSigned {
return lhs.bitWidth >= rhs.bitWidth ?
lhs < Self(truncatingIfNeeded: rhs) :
Other(truncatingIfNeeded: lhs) < rhs
}

return Other(truncatingIfNeeded: lhs) < rhs
// If `Self` is signed but `Other` is unsigned, then we have to
// be a little more careful about widening, since:
// (1) a fixed-width signed type can't represent the largest values of
// a fixed-width unsigned type of equal bit width; and
// (2) an unsigned type (obviously) can't represent a negative value.
if Self.isSigned {
return lhs.bitWidth > rhs.bitWidth ? // (1)
lhs < Self(truncatingIfNeeded: rhs) :
(lhs < (0 as Self) || Other(truncatingIfNeeded: lhs) < rhs) // (2)
}
// Analogous reasoning applies if `Other` is signed but `Self` is not.
return lhs.bitWidth < rhs.bitWidth ?
Other(truncatingIfNeeded: lhs) < rhs :
(rhs > (0 as Other) && lhs < Self(truncatingIfNeeded: rhs))
}

/// Returns a Boolean value indicating whether the value of the first
Expand Down
14 changes: 14 additions & 0 deletions test/IRGen/integer-comparison.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// RUN: %target-swift-frontend -primary-file %s -O -emit-ir | %FileCheck %s
// REQUIRES: CPU=x86_64 || CPU=arm64

func f(_ x: Int, _ y: UInt32) -> Bool { x < y }
// CHECK-LABEL: define {{.*}} @"$s4main1fySbSi_s6UInt32VtF"(i64 %0, i32 %1)
// CHECK: %2 = zext i32 %1 to i64
// CHECK-NEXT: %3 = icmp sgt i64 %2, %0
// CHECK-NEXT: ret i1 %3

func g(_ x: UInt32, _ y: Int) -> Bool { x < y }
// CHECK-LABEL: define {{.*}} @"$s4main1gySbs6UInt32V_SitF"(i32 %0, i64 %1)
// CHECK: %2 = zext i32 %0 to i64
// CHECK-NEXT: %3 = icmp slt i64 %2, %1
// CHECK-NEXT: ret i1 %3
38 changes: 16 additions & 22 deletions test/IRGen/temporary_allocation/codegen.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
// RUN: %target-swift-frontend -primary-file %s -O -emit-ir | %FileCheck %s --check-prefix=CHECK-%target-vendor
// RUN: %target-swift-frontend -primary-file %s -O -emit-ir | %FileCheck %s --check-prefixes=CHECK,CHECK-LARGE-ALLOC,CHECK-LARGE-ALLOC-%target-vendor
// RUN: %target-swift-frontend -primary-file %s -O -emit-ir | %FileCheck %s --check-prefix=CHECK-LARGE-STACK-ALLOC -DWORD=i%target-ptrsize
// RUN: %target-swift-frontend -primary-file %s -O -emit-ir | %FileCheck %s --check-prefix=CHECK-LARGE-HEAP-ALLOC -DWORD=i%target-ptrsize

@_silgen_name("blackHole")
func blackHole(_ value: UnsafeMutableRawPointer?) -> Void
Expand Down Expand Up @@ -74,24 +76,16 @@ withUnsafeTemporaryAllocation(of: Void.self, capacity: 2) { buffer in
withUnsafeTemporaryAllocation(byteCount: 0x0FFF_FFFF, alignment: 1) { buffer in
blackHole(buffer.baseAddress)
}
// CHECK-apple: [[IS_OS_OK:%[0-9]+]] = call swiftcc i1 @"$ss26_stdlib_isOSVersionAtLeastyBi1_Bw_BwBwtF"
// CHECK-apple: br i1 [[IS_OS_OK]], label %[[OS_OK_BR:[0-9]+]], label %[[UNSAFE_BR:[0-9]+]]

// CHECK-apple: [[UNSAFE_BR]]:
// CHECK-unknown: [[UNSAFE_BR:[0-9]+]]:
// CHECK: [[HEAP_PTR_RAW:%[0-9]+]] = call noalias i8* @swift_slowAlloc([[WORD]] 268435455, [[WORD]] -1)
// CHECK: [[HEAP_PTR:%[0-9]+]] = ptrtoint i8* [[HEAP_PTR_RAW]] to [[WORD]]
// CHECK: call swiftcc void @blackHole([[WORD]] [[HEAP_PTR]])
// CHECK: call void @swift_slowDealloc(i8* [[HEAP_PTR_RAW]], [[WORD]] -1, [[WORD]] -1)

// CHECK-apple: [[OS_OK_BR]]:
// CHECK: [[IS_SAFE:%[0-9]+]] = call zeroext i1 @swift_stdlib_isStackAllocationSafe([[WORD]] 268435455, [[WORD]] 1)
// CHECK: br i1 [[IS_SAFE]], label %[[SAFE_BR:[0-9]+]], label %[[UNSAFE_BR]]

// CHECK: [[SAFE_BR]]:
// CHECK: [[SPSAVE:%spsave[0-9]*]] = call i8* @llvm.stacksave()
// CHECK: [[STACK_PTR_RAW:%temp_alloc[0-9]*]] = alloca [268435455 x i8], align 1
// CHECK: [[STACK_PTR:%[0-9]+]] = ptrtoint [268435455 x i8]* [[STACK_PTR_RAW]] to [[WORD]]
// CHECK: call swiftcc void @blackHole([[WORD]] [[STACK_PTR]])
// CHECK: call void @llvm.stackrestore(i8* [[SPSAVE]])

// CHECK-LARGE-HEAP-ALLOC: [[HEAP_PTR_RAW:%[0-9]+]] = call noalias i8* @swift_slowAlloc([[WORD]] 268435455, [[WORD]] -1)
// CHECK-LARGE-HEAP-ALLOC-NEXT: [[HEAP_PTR:%[0-9]+]] = ptrtoint i8* [[HEAP_PTR_RAW]] to [[WORD]]
// CHECK-LARGE-HEAP-ALLOC-NEXT: call swiftcc void @blackHole([[WORD]] [[HEAP_PTR]])
// CHECK-LARGE-HEAP-ALLOC-NEXT: call void @swift_slowDealloc(i8* [[HEAP_PTR_RAW]], [[WORD]] -1, [[WORD]] -1)

// CHECK-LARGE-STACK-ALLOC: [[STACK_PTR_RAW:%temp_alloc[0-9]*]] = alloca [268435455 x i8], align 1
// CHECK-LARGE-STACK-ALLOC-NEXT: [[STACK_PTR:%[0-9]+]] = ptrtoint [268435455 x i8]* [[STACK_PTR_RAW]] to [[WORD]]
// CHECK-LARGE-STACK-ALLOC-NEXT: call swiftcc void @blackHole([[WORD]] [[STACK_PTR]])

// CHECK-LARGE-ALLOC-DAG: [[IS_SAFE:%[0-9]+]] = call zeroext i1 @swift_stdlib_isStackAllocationSafe([[WORD]] 268435455, [[WORD]] 1)
// CHECK-LARGE-ALLOC-DAG: br i1 [[IS_SAFE]], label %{{[0-9]+}}, label %{{[0-9]+}}
// CHECK-LARGE-ALLOC-apple-DAG: [[IS_OS_OK:%[0-9]+]] = call swiftcc i1 @"$ss26_stdlib_isOSVersionAtLeastyBi1_Bw_BwBwtF"
// CHECK-LARGE-ALLOC-apple-DAG: br i1 [[IS_OS_OK]], label %{{[0-9]+}}, label %{{[0-9]+}}