opened
gwynne
username_0
The `FixedPointConversion` validation test appears to have been completely hosted for quite some time. The following changes are made to restore it to (hopefully) intended operation:

- Use `lit.cfg`'s `%target-ptrsize` substitution instead of nesting `gyb` templates; this is faster, cleaner, and provides correct `line-directive` output.
- Floating-point/integer comparisons in Python were using string/int comparison, which has undefined behavior in Python 2 and is flat-out illegal in 3; this prevented a large number of tests from being generated at all. Replaced with proper numeric comparisons.
- Work around an edge case where the inability of Python to represent `Int64.min` exactly in floating-point due to insufficient precision resulted in the `gyb` logic choosing the wrong test logic, causing a spurious test failure.
- Fix an inverted assertion in the tests where `.init(exactly:)` returns non-`nil` but was being checked for `nil`.
- Respect the "round-towards-zero then check range" behavior of the truncating `.init(_:)` initializers; prevents incorrect test failures at the extreme ranges of large integer types.
Fix FixedPointConversion validation test
issue
created
gwynne
username_0
@swift-ci please test
comment
created
gottesmm
username_1
@username_0 I think that @username_2  should take a look at this as well. Once he is cool with the changes, I am happy to look at the python part.
comment
created
gwynne
username_0
I'm working on clearing up the failure on Linux; I solved the same issue for `Int64.min` but it seems `UInt64.max` exhibits it on Linux only for some reason...
comment
created
gwynne
username_0
@swift-ci please test
comment
created
gwynne
username_0
@username_2 Would very much appreciate your eyes on this one; the one test failure I'm getting on Linux _only_ pops up in the Release version of the test, works as expected in Debug:

```
[ RUN      ] FloatingPointToFixedPointConversionTruncations.Float80ToUInt64Conversion/dest=18446744073709551615
stdout>>> check failed at /home/username_0/swift/swift/validation-test/stdlib/Inputs/FixedPointConversion.swift.gyb, line 130
stdout>>> expected: 18446744073709551615.0 (of type Swift.Float80)
stdout>>> actual: 18446743523953737728.0 (of type Swift.Float80)
[     FAIL ] FloatingPointToFixedPointConversionTruncations.Float80ToUInt64Conversion/dest=18446744073709551615
[ RUN      ] FloatingPointToFixedPointConversionTruncations.Float80ToUIntConversion/dest=18446744073709551615
stdout>>> check failed at /home/username_0/swift/swift/validation-test/stdlib/Inputs/FixedPointConversion.swift.gyb, line 130
stdout>>> expected: 18446744073709551615.0 (of type Swift.Float80)
stdout>>> actual: 18446743523953737728.0 (of type Swift.Float80)
[     FAIL ] FloatingPointToFixedPointConversionTruncations.Float80ToUIntConversion/dest=18446744073709551615
````
There must be a reason the `Float80` -> `Uint[64]` conversion is losing precision only in optimizing mode on Linux, but I don't know what it is 😕
comment
created
gwynne
username_0
I'm running the individual test in question via:
```bash
~/swift/swift$ ../llvm/utils/lit/lit.py -a ../build/Ninja-RelWithDebInfoAssert/swift-linux-x86_64/validation-test-linux-x86_64/ --filter='stdlib/FixedPointConversion_Release.*'
```
comment
created
gwynne
username_0
@username_2 

[funcfoo_Debug.s.txt](https://github.com/apple/swift/files/2785479/funcfoo_Debug.s.txt)
[funcfoo_Release.s.txt](https://github.com/apple/swift/files/2785480/funcfoo_Release.s.txt)

See attached
comment
created
stephentyrone
username_2
@username_0 Thanks. I've been swamped the past few days, but I'll take a look this weekend.
comment
created
gwynne
username_0
@username_2 Ping 😅
comment
created
drodriguez
username_3
@username_0: I didn’t see this fix before, and I ended up fixing part of what you fix here in #23220. It mostly deals with the floating to integer conversions. It would be good to merge both solutions. I think your solution is missing some pieces like using `Decimal` to not loose precision in the Python side, and doing the right tests against the float to integer bounds (which are different than the integer bounds for unsigned types).

Tell me how you want to coordinate on this.
comment
created
gwynne
username_0
@username_3 I would suggest you integrate this PR into yours (to me the most critical change is getting rid of the unnecessary `gyb` nesting! 🙂) and go from there; you definitely know better how to handle the precision issues than I do.
comment
created
stephentyrone
username_2
@username_0 @username_3 Ping me once you two decide how to handle the overlapping PRs and I'll review and merge them.
comment
created
CodaFi
username_4
Looks like we took #23220.  Closing this out.  Thanks y'all!
comment