[stdlib] Reinstate the FixedPointConversion tests #36205

benrimmington · 2021-02-28T02:00:03Z

Follow-up to: #23524

I've tested on an Intel-based Mac (without Float16):

utils/run-test \
  --build-dir ../build/Ninja-RelWithDebInfoAssert \
  --verbose \
  validation-test/stdlib/FixedPointConversion/*.swift

********************

Testing Time: 17.22s
  Unsupported: 20
  Passed     : 20

Resolves: rdar://problem/49177883

benrimmington · 2021-02-28T02:04:52Z

@swift-ci Please test

swift-ci · 2021-02-28T03:25:17Z

Build failed
Swift Test Linux Platform
Git Sha - 42a6782

swift-ci · 2021-02-28T04:47:11Z

Build failed
Swift Test OS X Platform
Git Sha - 42a6782

benrimmington · 2021-02-28T14:29:03Z

Linux with preset buildbot_linux_1604 (includes --long-test --stress-test):

******************** TEST 'Swift(linux-x86_64) :: stdlib/FixedPointConversion_{Debug,Release}.test-sh' FAILED ********************

[ RUN      ] FloatingPointConversionTraps.Float16ToUInt32Conversion/dest=-1.1
expecting a crash, but the test did not crash
[     FAIL ] FloatingPointConversionTraps.Float16ToUInt32Conversion/dest=-1.1
[ RUN      ] FloatingPointConversionTraps.Float16ToUInt32Conversion/dest=-2047
expecting a crash, but the test did not crash
[     FAIL ] FloatingPointConversionTraps.Float16ToUInt32Conversion/dest=-2047
[ RUN      ] FloatingPointConversionTraps.UInt32/Float16/negative
expecting a crash, but the test did not crash
[     FAIL ] FloatingPointConversionTraps.UInt32/Float16/negative

[ RUN      ] FloatingPointConversionTraps.Float16ToUInt64Conversion/dest=-1.1
expecting a crash, but the test did not crash
[     FAIL ] FloatingPointConversionTraps.Float16ToUInt64Conversion/dest=-1.1
[ RUN      ] FloatingPointConversionTraps.Float16ToUInt64Conversion/dest=-2047
expecting a crash, but the test did not crash
[     FAIL ] FloatingPointConversionTraps.Float16ToUInt64Conversion/dest=-2047
[ RUN      ] FloatingPointConversionTraps.UInt64/Float16/negative
expecting a crash, but the test did not crash
[     FAIL ] FloatingPointConversionTraps.UInt64/Float16/negative

[ RUN      ] FloatingPointConversionTraps.Float16ToUIntConversion/dest=-1.1
expecting a crash, but the test did not crash
[     FAIL ] FloatingPointConversionTraps.Float16ToUIntConversion/dest=-1.1
[ RUN      ] FloatingPointConversionTraps.Float16ToUIntConversion/dest=-2047
expecting a crash, but the test did not crash
[     FAIL ] FloatingPointConversionTraps.Float16ToUIntConversion/dest=-2047
[ RUN      ] FloatingPointConversionTraps.UInt/Float16/negative
expecting a crash, but the test did not crash
[     FAIL ] FloatingPointConversionTraps.UInt/Float16/negative

Failed Tests (2):
  Swift(linux-x86_64) :: stdlib/FixedPointConversion_Debug.test-sh
  Swift(linux-x86_64) :: stdlib/FixedPointConversion_Release.test-sh

benrimmington · 2021-02-28T14:32:29Z

macOS with preset buildbot_all_platforms,tools=RA,stdlib=RA:

UNSUPPORTED: Swift(macosx-x86_64) :: stdlib/FixedPointConversion_Release.test-sh (810 of 14265)
UNSUPPORTED: Swift(macosx-x86_64) :: stdlib/FixedPointConversion_Debug.test-sh (811 of 14265)

UNSUPPORTED: Swift(iphonesimulator-x86_64) :: stdlib/FixedPointConversion_Release.test-sh (810 of 14265)
UNSUPPORTED: Swift(iphonesimulator-x86_64) :: stdlib/FixedPointConversion_Debug.test-sh (811 of 14265)

UNSUPPORTED: Swift(watchsimulator-i386) :: stdlib/FixedPointConversion_Release.test-sh (810 of 14265)
UNSUPPORTED: Swift(watchsimulator-i386) :: stdlib/FixedPointConversion_Debug.test-sh (811 of 14265)

Failed Tests (2):
  Swift(watchsimulator-i386) :: Interop/Cxx/templates/class-template-uninstantiatable-members-irgen.swift
  Swift(watchsimulator-i386) :: DebugInfo/async-let-await.swift

benrimmington · 2021-03-01T15:03:13Z

The following Float16 to UInt{32,64} conversions aren't crashing as expected.

// EXPECTED: Fatal error: Float16 value cannot be converted to UInt32
//           because the result would be less than UInt32.min

// FloatingPointConversionTraps.Float16ToUInt32Conversion/dest=-1.1
UInt32(Float16(-1.1))     //-> ACTUAL: UInt32.max

// FloatingPointConversionTraps.Float16ToUInt32Conversion/dest=-2047
UInt32(Float16(-2047.0))  //-> ACTUAL: (UInt32.max - 2046)

// FloatingPointConversionTraps.UInt32/Float16/negative
UInt32(Float16(-123.0))   //-> ACTUAL: (UInt32.max - 122)

These expected and actual results are from an Xcode Playground (iOS or tvOS).
The pull request can only be tested on Linux, due to REQUIRES: long_test.

xwu · 2021-03-01T23:52:44Z

The following Float16 to UInt{32,64} conversions aren't crashing as expected.

// EXPECTED: Fatal error: Float16 value cannot be converted to UInt32
//           because the result would be less than UInt32.min

// FloatingPointConversionTraps.Float16ToUInt32Conversion/dest=-1.1
UInt32(Float16(-1.1))     //-> ACTUAL: UInt32.max

// FloatingPointConversionTraps.Float16ToUInt32Conversion/dest=-2047
UInt32(Float16(-2047.0))  //-> ACTUAL: (UInt32.max - 2046)

// FloatingPointConversionTraps.UInt32/Float16/negative
UInt32(Float16(-123.0))   //-> ACTUAL: (UInt32.max - 122)

These expected and actual results are from an Xcode Playground (iOS or tvOS).
The pull request can only be tested on Linux, due to REQUIRES: long_test.

🤔

This doesn't make sense to me, and in more than one way:

First, on my actual iPad with the release version of Swift Playgrounds, it says:

UInt32(Float16(-1.1)) // Error: Negative literal '-1.1' cannot be converted to 'UInt32'

This behaves as expected and doesn't line up with what you're seeing.

Second, this also shouldn't be testing floating-point conversions: as the diagnostic shows, since SE-0213, literal initialization behaves like coercion.

Third, curiously, testing actual conversions on the iPad shows that something's not right:

let x = -1.1 as Double
UInt32(Float16(x)) // 0

let y = -1.1 as Float16
UInt32(y) // 0 -- Uh oh...

xwu · 2021-03-01T23:59:12Z

Aha! I have found the think-o. And I don't think I'm to blame for this one :)

benrimmington · 2021-03-02T12:45:05Z

First, on my actual iPad with the release version of Swift Playgrounds, it says:
UInt32(Float16(-1.1)) // Error: Negative literal '-1.1' cannot be converted to 'UInt32'
This behaves as expected and doesn't line up with what you're seeing.

In an Xcode 12.4 Playground (iOS simulated), I'm only seeing that diagnostic when using integer literals with Float16:

UInt32(Float16(-123.0))   //-> 4294967173
UInt32(Float16(-123))     //-> error: Negative literal '-123' cannot be converted to 'UInt32'

Second, this also shouldn't be testing floating-point conversions: as the diagnostic shows, since SE-0213, literal initialization behaves like coercion.

I'm seeing the same results either way, and also for init(exactly:) with some inputs:

let x = Float16(-1.1)     //-> -1.1
let y = Float16(-2047.0)  //-> -2047.0
let z = Float16(-123.0)   //-> -123.0

UInt32(x)                 //-> 4294967295
UInt32(exactly: x)        //-> nil

UInt32(y)                 //-> 4294965249
UInt32(exactly: y)        //-> 4294965249

UInt32(z)                 //-> 4294967173
UInt32(exactly: z)        //-> 4294967173

It's possible that the description, debugDescription, or playgroundDescription for Float16 is also to blame.

stephentyrone · 2021-03-02T14:00:11Z

@swift-ci please test

benrimmington · 2021-03-02T15:22:15Z

I would expect the same failures for init(exactly:), but the previous job didn't contain any results for the "FloatingPointToFixedPointConversionFailures" test suite.

swift-ci · 2021-03-02T15:38:52Z

Build failed
Swift Test Linux Platform
Git Sha - 42a6782

xwu · 2021-03-02T15:42:01Z

Unrelated test failure on Linux:

The following tests FAILED:
10:38:47 1091 - TestFoundation.TestOperationQueue-test_CustomOperationReady (Failed)

Let’s try again...

@swift-ci please test Linux platform

xwu · 2021-03-02T15:43:20Z

@benrimmington Sorry, in case it wasn’t clear, the cause of the test failures is now identified and fixed by #36219.

benrimmington · 2021-03-02T16:07:11Z

@xwu Thanks, I've taken a look at your pull request. I was actually referring to the test results from 3 days ago. There were failures for UInt{,32,64}.init(), but there weren't any results (success or failure) for UInt{,32,64}.init(exactly:). The file contains 5 test suites, but there were only results for 4 of them.

benrimmington · 2021-03-02T19:20:20Z

All checks have passed. However, the tests are among the slowest, and are currently unsupported on macOS CI. Shall I try to split the test suites into separate files?

--- check-swift-all-linux-x86_64 ---
(5th Slowest Test:)
253.09s: Swift(linux-x86_64) :: stdlib/FixedPointConversion_Release.test-sh

--- check-swift-all-optimize-linux-x86_64 ---
(3rd and 18th Slowest Tests:)
197.49s: Swift(linux-x86_64) :: stdlib/FixedPointConversion_Release.test-sh
 25.03s: Swift(linux-x86_64) :: stdlib/FixedPointConversion_Debug.test-sh

xwu · 2021-03-03T00:15:52Z

@benrimmington Yikes, agree it's probably a good idea to split, but why is it so slow in the first place?

benrimmington · 2021-03-03T10:59:00Z

@xwu The expanded gyb file contains 1768 tests. On macOS, using the swiftc -driver-time-compilation flag:

Debug: 86% of testing time is compilation time.

===-------------------------------------------------------------------------===
                            Driver Compilation Time
===-------------------------------------------------------------------------===
  Total Execution Time: 0.0002 seconds (45.5747 wall clock)

   ---User Time---   --System Time--   --User+System--   ---Wall Time---  --- Name ---
   0.0000 ( 73.5%)   0.0001 ( 68.3%)   0.0001 ( 69.7%)  45.4577 ( 99.7%)  {compile: FixedPointConversion-72cd1d.o <= FixedPointConversion.swift}
   0.0000 ( 26.5%)   0.0000 ( 31.7%)   0.0001 ( 30.3%)   0.1170 (  0.3%)  {link: a.out_Debug <= FixedPointConversion-72cd1d.o}
   0.0000 (100.0%)   0.0001 (100.0%)   0.0002 (100.0%)  45.5747 (100.0%)  Total

********************

Testing Time: 53.07s
  Passed: 1

Release: 92% of testing time is compilation time.

===-------------------------------------------------------------------------===
                            Driver Compilation Time
===-------------------------------------------------------------------------===
  Total Execution Time: 0.0002 seconds (79.9013 wall clock)

   ---User Time---   --System Time--   --User+System--   ---Wall Time---  --- Name ---
   0.0000 ( 71.4%)   0.0001 ( 73.1%)   0.0001 ( 72.6%)  79.7863 ( 99.9%)  {compile: FixedPointConversion-7bf5fa.o <= FixedPointConversion.swift}
   0.0000 ( 28.6%)   0.0000 ( 26.9%)   0.0000 ( 27.4%)   0.1149 (  0.1%)  {link: a.out_Release <= FixedPointConversion-7bf5fa.o}
   0.0000 (100.0%)   0.0001 (100.0%)   0.0002 (100.0%)  79.9013 (100.0%)  Total

********************

Testing Time: 86.68s
  Passed: 1

xwu · 2021-03-04T01:45:49Z

I see--thanks. Was worried that certain conversions were running unusually slowly.

benrimmington · 2021-03-08T03:58:13Z

@swift-ci Please test

benrimmington · 2021-03-08T07:14:10Z

https://ci.swift.org/job/swift-PR-Linux/26014/

Release configuration tests are now 5.5x to 7x faster on Linux, by splitting the GYB output into smaller files, and building with the -whole-module-optimization flag.

--- check-swift-all-linux-x86_64 ---
(20th Slowest Test:)
35.64s: Swift(linux-x86_64) :: stdlib/FixedPointConversion_Release.test-sh

--- check-swift-all-optimize-linux-x86_64 ---
(15th and 20th Slowest Tests:)
35.76s: Swift(linux-x86_64) :: stdlib/FixedPointConversion_Release.test-sh
21.23s: Swift(linux-x86_64) :: stdlib/FixedPointConversion_Debug.test-sh

(I need to adjust some test values in the GYB template.)

benrimmington · 2021-03-08T08:29:34Z

https://ci.swift.org/job/swift-PR-macos/26050/

The testing times are too slow on macOS and simulators.

(3rd and 4th Slowest Tests:)
169.07s: Swift(macosx-x86_64) :: stdlib/FixedPointConversion_Release.test-sh
148.33s: Swift(macosx-x86_64) :: stdlib/FixedPointConversion_Debug.test-sh

(2nd and 11th Slowest Tests:)
100.97s: Swift(iphonesimulator-x86_64) :: stdlib/FixedPointConversion_Release.test-sh
 55.67s: Swift(iphonesimulator-x86_64) :: stdlib/FixedPointConversion_Debug.test-sh

(1st and 10th Slowest Tests:)
538.47s: Swift(watchsimulator-i386) :: stdlib/FixedPointConversion_Release.test-sh
 68.41s: Swift(watchsimulator-i386) :: stdlib/FixedPointConversion_Debug.test-sh

benrimmington · 2021-03-09T02:37:15Z

@swift-ci Please test

swift-ci · 2021-03-09T04:29:21Z

Build failed
Swift Test Linux Platform
Git Sha - 7641890

swift-ci · 2021-03-09T06:26:17Z

Build failed
Swift Test OS X Platform
Git Sha - 7641890

benrimmington · 2021-03-09T08:33:42Z

The testing times are better, now that the 20 generated files are built and run in parallel.

They aren't listed under "Slowest Tests" for x86_64 (Linux, macOS, and iOS simulator).
For watchsimulator-i386, the -O (not -Onone) files are all listed, and smaller integer types are slower.

Slowest Tests:
--------------------------------------------------------------------------
595.20s: Swift(watchsimulator-i386) :: Casting/BoxingCasts-5.test
594.44s: Swift(watchsimulator-i386) :: Casting/BoxingCasts-4.test
128.25s: Swift(watchsimulator-i386) :: stdlib/CharacterPropertiesLong.swift
109.32s: Swift(watchsimulator-i386) :: Interpreter/dynamic_replacement.swift
108.95s: Swift(watchsimulator-i386) :: ParseableInterface/verify_all_overlays.py
97.64s: Swift(watchsimulator-i386) :: stdlib/FixedPointConversion/FixedPointConversion_Release32_ToInt8.swift
94.62s: Swift(watchsimulator-i386) :: stdlib/FixedPointConversion/FixedPointConversion_Release32_ToUInt8.swift
90.03s: Swift(watchsimulator-i386) :: stdlib/FixedPointConversion/FixedPointConversion_Release32_ToUInt16.swift
89.12s: Swift(watchsimulator-i386) :: stdlib/FixedPointConversion/FixedPointConversion_Release32_ToInt16.swift
86.03s: Swift(watchsimulator-i386) :: SIL/Serialization/deserialize_stdlib.sil
82.29s: Swift(watchsimulator-i386) :: stdlib/FixedPointConversion/FixedPointConversion_Release32_ToUInt.swift
81.81s: Swift(watchsimulator-i386) :: IDE/complete_value_expr.swift
81.62s: Swift(watchsimulator-i386) :: stdlib/FixedPointConversion/FixedPointConversion_Release32_ToUInt32.swift
79.75s: Swift(watchsimulator-i386) :: stdlib/FixedPointConversion/FixedPointConversion_Release32_ToInt32.swift
79.39s: Swift(watchsimulator-i386) :: stdlib/FixedPointConversion/FixedPointConversion_Release32_ToInt.swift
77.27s: Swift(watchsimulator-i386) :: stdlib/FixedPoint.swift.gyb
76.33s: Swift(watchsimulator-i386) :: stdlib/FixedPointConversion/FixedPointConversion_Release32_ToUInt64.swift
69.43s: Swift(watchsimulator-i386) :: stdlib/FixedPointConversion/FixedPointConversion_Release32_ToInt64.swift
56.18s: Swift(watchsimulator-i386) :: stdlib/StringMemoryTest.swift
53.77s: Swift(watchsimulator-i386) :: stdlib/NumericParsing.swift.gyb

benrimmington · 2021-03-09T11:11:14Z

@swift-ci Please test

benrimmington · 2021-03-10T01:02:22Z

@stephentyrone I believe this is ready for review.

The GYB template and generated files have been moved into a subdirectory.
Some generated files require long_test, due to slow i386 optimizations.
The .crashOutputMatches checks weren't working, so I've removed them for now.
I'm not sure if getFtoIBounds is correctly implemented, and it probably isn't needed here.
I've tested with some hard-coded values (e.g. -128.5, +127.5) suitable for all floating-point types.
The old tests were using values outside the exclusive bounds.

stephentyrone · 2021-03-10T03:22:58Z

These can obviously be sped up quite a bit more, but I'm fine with taking them as is for now.

[stdlib] Reinstate the FixedPointConversion tests

42a6782

benrimmington marked this pull request as draft February 28, 2021 14:36

benrimmington requested review from lorentey and stephentyrone March 1, 2021 15:00

xwu mentioned this pull request Mar 2, 2021

[stdlib] Fix conversion from Float16 to unsigned integer types #36219

Merged

[stdlib] Update the FixedPointConversion tests

85e0615

[stdlib] Generate the FixedPointConversion tests

7641890

[stdlib] FixedPointConversion_Release32: long_test

839d6fc

benrimmington marked this pull request as ready for review March 10, 2021 00:57

stephentyrone approved these changes Mar 10, 2021

View reviewed changes

stephentyrone merged commit d2495f8 into swiftlang:main Mar 10, 2021

benrimmington deleted the rdar49177883 branch March 10, 2021 03:26

benrimmington mentioned this pull request Mar 13, 2021

[stdlib] Speed up the FixedPointConversion tests #36434

Merged

[stdlib] Reinstate the FixedPointConversion tests #36205

[stdlib] Reinstate the FixedPointConversion tests #36205

Uh oh!

Conversation

benrimmington commented Feb 28, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

benrimmington commented Feb 28, 2021

Uh oh!

swift-ci commented Feb 28, 2021

Uh oh!

swift-ci commented Feb 28, 2021

Uh oh!

benrimmington commented Feb 28, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

benrimmington commented Feb 28, 2021

Uh oh!

benrimmington commented Mar 1, 2021

Uh oh!

xwu commented Mar 1, 2021

Uh oh!

xwu commented Mar 1, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

benrimmington commented Mar 2, 2021

Uh oh!

stephentyrone commented Mar 2, 2021

Uh oh!

benrimmington commented Mar 2, 2021

Uh oh!

swift-ci commented Mar 2, 2021

Uh oh!

xwu commented Mar 2, 2021

Uh oh!

xwu commented Mar 2, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

benrimmington commented Mar 2, 2021

Uh oh!

benrimmington commented Mar 2, 2021

Uh oh!

xwu commented Mar 3, 2021

Uh oh!

benrimmington commented Mar 3, 2021

Uh oh!

xwu commented Mar 4, 2021

Uh oh!

benrimmington commented Mar 8, 2021

Uh oh!

benrimmington commented Mar 8, 2021

Uh oh!

benrimmington commented Mar 8, 2021

Uh oh!

benrimmington commented Mar 9, 2021

Uh oh!

swift-ci commented Mar 9, 2021

Uh oh!

swift-ci commented Mar 9, 2021

Uh oh!

benrimmington commented Mar 9, 2021

Uh oh!

benrimmington commented Mar 9, 2021

Uh oh!

benrimmington commented Mar 10, 2021

Uh oh!

stephentyrone commented Mar 10, 2021

Uh oh!

Uh oh!

benrimmington commented Feb 28, 2021 •

edited

Loading

benrimmington commented Feb 28, 2021 •

edited

Loading

xwu commented Mar 1, 2021 •

edited

Loading

xwu commented Mar 2, 2021 •

edited

Loading