Merge pull request #13007 from xwu/fused-multiply-add-stride

[stdlib] Eliminate intermediate rounding error in floating-point strides (and related gardening)

Author: moiseev

stdlib/public/core/Range.swift.gyb

@@ -80,13 +80,6 @@ extension RangeExpression {
   }  
 }
 
-// FIXME(ABI)#55 (Statically Unavailable/Dynamically Available): remove this
-// type, it creates an ABI burden on the library.
-//
-// A dummy type that we can use when we /don't/ want to create an
-// ambiguity indexing CountableRange<T> outside a generic context.
-public enum _DisabledRangeIndex_ {}
-
 /// A half-open range that forms a collection of consecutive values.
 ///
 /// You create a `CountableRange` instance by using the half-open range

stdlib/public/core/Stride.swift.gyb

@@ -43,7 +43,7 @@ public protocol Strideable : Comparable {
     from start: Self, by distance: Self.Stride
   ) -> (index: Int?, value: Self)
 
-  associatedtype _DisabledRangeIndex = _DisabledRangeIndex_
+  associatedtype _DisabledRangeIndex = Never
 }
 
 extension Strideable {
@@ -137,13 +137,27 @@ extension Strideable where Stride : FloatingPoint {
     from start: Self, by distance: Self.Stride
   ) -> (index: Int?, value: Self) {
     if let i = current.index {
+      // When Stride is a floating-point type, we should avoid accumulating
+      // rounding error from repeated addition.
       return (i + 1, start.advanced(by: Stride(i + 1) * distance))
     }
-    // If current.index == nil, either we're just starting out (in which case
-    // the next index is 1), or we should proceed without an index just as
-    // though this floating point specialization doesn't exist.
-    return (current.value == start ? 1 : nil,
-            current.value.advanced(by: distance))
+    return (nil, current.value.advanced(by: distance))
+  }
+}
+
+extension Strideable where Self : FloatingPoint, Self == Stride {
+  @_inlineable
+  public static func _step(
+    after current: (index: Int?, value: Self),
+    from start: Self, by distance: Self.Stride
+  ) -> (index: Int?, value: Self) {
+    if let i = current.index {
+      // When both Self and Stride are the same floating-point type, we should
+      // take advantage of fused multiply-add (where supported) to eliminate
+      // intermediate rounding error.
+      return (i + 1, start.addingProduct(Stride(i + 1), distance))
+    }
+    return (nil, current.value.advanced(by: distance))
   }
 }
 
@@ -168,7 +182,7 @@ public struct StrideToIterator<Element : Strideable> : IteratorProtocol {
     self._start = _start
     _end = end
     _stride = stride
-    _current = (nil, _start)
+    _current = (0, _start)
   }
 
   /// Advances to the next element and returns it, or `nil` if no next element
@@ -255,111 +269,52 @@ public struct StrideTo<Element : Strideable> : Sequence, CustomReflectable {
   }
 }
 
-// FIXME(conditional-conformances): these extra types can easily be turned into
-// conditional extensions to StrideTo type
-% for Self, ElementConstraint, Where in [
-%   ('IntegerStrideToCollection', 'BinaryInteger', 'Element.Stride : BinaryInteger'),
-%   ('FloatingPointStrideToCollection', 'BinaryFloatingPoint', 'Element.Stride == Element'),
-% ]:
-%   ElementIsInteger = ElementConstraint == 'BinaryInteger'
-
-internal struct ${Self}<
-  Element : ${ElementConstraint}
-> : RandomAccessCollection, CustomReflectable
-where ${Where} {
-
-//===----------------------------------------------------------------------===//
-// This block is copied from StrideTo struct definition                       //
-//===----------------------------------------------------------------------===//
-  @_inlineable
-  public func makeIterator() -> StrideToIterator<Element> {
-    return StrideToIterator(_start: _start, end: _end, stride: _stride)
-  }
-
-  @_inlineable
-  public func _customContainsEquatableElement(
-    _ element: Element
-  ) -> Bool? {
-    if element < _start || _end <= element {
-      return false
-    }
-    return nil
-  }
-
-  @_inlineable
-  @_versioned
-  internal init(_start: Element, end: Element, stride: Element.Stride) {
-    _precondition(stride != 0, "Stride size must not be zero")
-    // At start, striding away from end is allowed; it just makes for an
-    // already-empty Sequence.
-    self._start = _start
-    self._end = end
-    self._stride = stride
-  }
-
-  @_versioned
-  internal let _start: Element
-
-  @_versioned
-  internal let _end: Element
-
-  @_versioned
-  internal let _stride: Element.Stride
-
-  @_inlineable // FIXME(sil-serialize-all)
-  public var customMirror: Mirror {
-    return Mirror(self, children: ["from": _start, "to": _end, "by": _stride])
-  }
-//===----------------------------------------------------------------------===//
-// The end of the copied block
-//===----------------------------------------------------------------------===//
-
-  // RandomAccessCollection conformance
+// FIXME(conditional-conformances): This does not yet compile (SR-6474).
+#if false
+extension StrideTo : RandomAccessCollection
+where Element.Stride : BinaryInteger {
   public typealias Index = Int
-  public typealias SubSequence = RandomAccessSlice<${Self}>
+  public typealias SubSequence = RandomAccessSlice<StrideTo<Element>>
   public typealias Indices = CountableRange<Int>
 
+  @_inlineable
   public var startIndex: Index { return 0 }
+
+  @_inlineable
   public var endIndex: Index { return count }
 
+  @_inlineable
   public var count: Int {
-    let (start, end, stride) =
-      (_stride > 0) ? (_start, _end, _stride) : (_end, _start, -_stride)
-%   if ElementIsInteger:
-    return Int((start.distance(to: end) - 1) / stride) + 1
-%   else:
-    let nonExactCount = (start.distance(to: end)) / stride
-    return Int(nonExactCount.rounded(.toNearestOrAwayFromZero))
-%   end
+    let distance = _start.distance(to: _end)
+    guard distance != 0 && (distance < 0) == (_stride < 0) else { return 0 }
+    return Int((distance - 1) / _stride) + 1
   }
 
   public subscript(position: Index) -> Element {
-    _failEarlyRangeCheck(position, bounds: startIndex ..< endIndex)
-    return _indexToElement(position)
+    _failEarlyRangeCheck(position, bounds: startIndex..<endIndex)
+    return _start.advanced(by: Element.Stride(position) * _stride)
   }
 
-  public subscript(bounds: Range<Index>) -> RandomAccessSlice<${Self}> {
-    _failEarlyRangeCheck(bounds, bounds: startIndex ..< endIndex)
+  public subscript(
+    bounds: Range<Index>
+  ) -> RandomAccessSlice<StrideTo<Element>> {
+    _failEarlyRangeCheck(bounds, bounds: startIndex..<endIndex)
     return RandomAccessSlice(base: self, bounds: bounds)
   }
 
-  public func index(after i: Index) -> Index {
-    _failEarlyRangeCheck(i, bounds: startIndex-1 ..< endIndex)
-    return i+1
-  }
-
+  @_inlineable
   public func index(before i: Index) -> Index {
-    _failEarlyRangeCheck(i, bounds: startIndex+1 ... endIndex)
-    return i-1
+    _failEarlyRangeCheck(i, bounds: startIndex + 1...endIndex)
+    return i - 1
   }
 
-  @inline(__always)
-  internal func _indexToElement(_ i: Index) -> Element {
-    return _start.advanced(by: Element.Stride(i) * _stride)
+  @_inlineable
+  public func index(after i: Index) -> Index {
+    _failEarlyRangeCheck(i, bounds: startIndex - 1..<endIndex)
+    return i + 1
   }
 }
-
-% end
+#endif
 
 /// Returns the sequence of values (`self`, `self + stride`, `self +
 /// 2 * stride`, ... *last*) where *last* is the last value in the
@@ -395,7 +350,7 @@ public struct StrideThroughIterator<Element : Strideable> : IteratorProtocol {
     self._start = _start
     _end = end
     _stride = stride
-    _current = (nil, _start)
+    _current = (0, _start)
   }
 
   /// Advances to the next element and returns it, or `nil` if no next element
@@ -487,93 +442,41 @@ public struct StrideThrough<
   }
 }
 
-// FIXME(conditional-conformances): these extra types can easily be turned into
-// conditional extensions to StrideThrough type
-% for Self, ElementConstraint, Where in [
-%   ('IntegerStrideThroughCollection', 'BinaryInteger', 'Element.Stride : BinaryInteger'),
-%   ('FloatingPointStrideThroughCollection', 'BinaryFloatingPoint', 'Element.Stride == Element'),
-% ]:
-%   ElementIsInteger = ElementConstraint == 'BinaryInteger'
-
-internal struct ${Self}<
-  Element : ${ElementConstraint}
-> : RandomAccessCollection, CustomReflectable
-where ${Where} {
-
-//===----------------------------------------------------------------------===//
-// This block is copied from StrideThrough struct definition                  //
-//===----------------------------------------------------------------------===//
-  /// Returns an iterator over the elements of this sequence.
-  ///
-  /// - Complexity: O(1).
-  @_inlineable
-  public func makeIterator() -> StrideThroughIterator<Element> {
-    return StrideThroughIterator(_start: _start, end: _end, stride: _stride)
-  }
-
-  @_inlineable
-  public func _customContainsEquatableElement(
-    _ element: Element
-  ) -> Bool? {
-    if element < _start || _end < element {
-      return false
-    }
-    return nil
-  }
-
-  @_inlineable
-  @_versioned
-  internal init(_start: Element, end: Element, stride: Element.Stride) {
-    _precondition(stride != 0, "Stride size must not be zero")
-    self._start = _start
-    self._end = end
-    self._stride = stride
-  }
-
-  @_versioned
-  internal let _start: Element
-  @_versioned
-  internal let _end: Element
-  @_versioned
-  internal let _stride: Element.Stride
-
-  @_inlineable // FIXME(sil-serialize-all)
-  public var customMirror: Mirror {
-    return Mirror(self,
-      children: ["from": _start, "through": _end, "by": _stride])
-  }
-//===----------------------------------------------------------------------===//
-// The end of the copied block
-//===----------------------------------------------------------------------===//
-
-  // RandomAccessCollection conformance
+// FIXME(conditional-conformances): This does not yet compile (SR-6474).
+#if false
+extension StrideThrough : RandomAccessCollection
+where Element.Stride : BinaryInteger {
   public typealias Index = ClosedRangeIndex<Int>
   public typealias IndexDistance = Int
-  public typealias SubSequence = RandomAccessSlice<${Self}>
+  public typealias SubSequence = RandomAccessSlice<StrideThrough<Element>>
 
   @_inlineable
-  public var startIndex: Index { return ClosedRangeIndex(0) }
+  public var startIndex: Index {
+    let distance = _start.distance(to: _end)
+    return distance == 0 || (distance < 0) == (_stride < 0)
+      ? ClosedRangeIndex(0)
+      : ClosedRangeIndex()
+  }
+
   @_inlineable
   public var endIndex: Index { return ClosedRangeIndex() }
 
   @_inlineable
   public var count: Int {
-    let (start, end, stride) =
-      (_stride > 0) ? (_start, _end, _stride) : (_end, _start, -_stride)
-%   if ElementIsInteger:
-    return Int(start.distance(to: end) / stride) + 1
-%   else:
-    let nonExactCount = start.distance(to: end) / stride
-    return Int(nonExactCount.rounded(.toNearestOrAwayFromZero)) + 1
-%   end
+    let distance = _start.distance(to: _end)
+    guard distance != 0 else { return 1 }
+    guard (distance < 0) == (_stride < 0) else { return 0 }
+    return Int(distance / _stride) + 1
   }
 
   public subscript(position: Index) -> Element {
     let offset = Element.Stride(position._dereferenced) * _stride
     return _start.advanced(by: offset)
   }
 
-  public subscript(bounds: Range<Index>) -> RandomAccessSlice<${Self}> {
+  public subscript(
+    bounds: Range<Index>
+  ) -> RandomAccessSlice<StrideThrough<Element>> {
     return RandomAccessSlice(base: self, bounds: bounds)
   }
 
@@ -601,8 +504,7 @@ where ${Where} {
     }
   }
 }
-
-% end
+#endif
 
 /// Returns the sequence of values (`self`, `self + stride`, `self +
 /// 2 * stride`, ... *last*) where *last* is the last value in the
@@ -615,4 +517,3 @@ public func stride<T>(
 ) -> StrideThrough<T> {
   return StrideThrough(_start: start, end: end, stride: stride)
 }
-

test/stdlib/Strideable.swift

@@ -188,12 +188,20 @@ StrideTestSuite.test("FloatingPointStride") {
   expectEqual([ 1.4, 2.4, 3.4 ], result)
 }
 
-StrideTestSuite.test("ErrorAccumulation") {
-  let a = Array(stride(from: Float(1.0), through: Float(2.0), by: Float(0.1)))
+StrideTestSuite.test("FloatingPointStride/rounding error") {
+  // Ensure that there is no error accumulation
+  let a = Array(stride(from: 1 as Float, through: 2, by: 0.1))
   expectEqual(11, a.count)
-  expectEqual(Float(2.0), a.last)
-  let b = Array(stride(from: Float(1.0), to: Float(10.0), by: Float(0.9)))
+  expectEqual(2 as Float, a.last)
+  let b = Array(stride(from: 1 as Float, to: 10, by: 0.9))
   expectEqual(10, b.count)
+
+  // Ensure that there is no intermediate rounding error on supported platforms
+  if (-0.2).addingProduct(0.2, 6) == 1 {
+    let c = Array(stride(from: -0.2, through: 1, by: 0.2))
+    expectEqual(7, c.count)
+    expectEqual(1 as Double, c.last)
+  }
 }
 
 func strideIteratorTest<