Skip to content

Conversation

stephentyrone
Copy link
Contributor

Updated SIMD implementation, based on feedback from first round of review.

This approach, suggested by Chris, is probably more flexible in the long run, but in the short-term we'll have some fairly ugly codegen with -Onone (-O builds are mostly OK except where masks come into it). We'll need some @_semantics wizardry to make this really work, but it definitely lets us have significantly less boilerplate going forward.

Under discussion as SE-0229

@rudkx
Copy link
Contributor

rudkx commented Nov 8, 2018

Can you make sure to test the validation tests of a release no-assertions build to ensure they pass after the new overloads are added?

Specifically the tests in validation-test/Sema/type_checker_perf?

Some of those run on one of the builders but do not get tested by the normal PR testing (other tests, the ones based on scale-test, do run in the PR testing).

@stephentyrone
Copy link
Contributor Author

stephentyrone commented Nov 8, 2018

@rudkx Yup. There's a little bit of additional cleanup to do and I want to rebase on @rxwei's AdditiveArithmetic changes, since those also tie into this, and then I'll start in on that.

@stephentyrone
Copy link
Contributor Author

@swift-ci please test

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - b72195a09296816363e059bbebc80b16dcf1f34c

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - fe5af74cf40cbe5c611f2a74e6ef53b8bca04b38

@lattner
Copy link
Contributor

lattner commented Nov 14, 2018

@rxwei FYI, this is the updated patch with some renamings to consider if you are interested.

@rxwei
Copy link
Contributor

rxwei commented Nov 14, 2018

The proposal hasn't been updated to reflect the implementation. I have comments (many, i think) but I'd like to quote contents of the proposal instead of the PR.

Also changes elementCount to scalarCount, per review thread.
Move all SIMD-associated things under SIMD names, both for clarity and so we can use the name Vector for math vectors at some future point if we want.

- Vectorizable -> SIMDScalar
- SIMDVector -> SIMD
- SIMDMaskVector -> SIMDMask
- VectorStorage -> SIMDStorage
- Vector${N} -> SIMD${N}

This also removes the SIMDMask protocol entirely, leaving only the concrete type, by reworking how the mask associated type works.
@stephentyrone
Copy link
Contributor Author

@swift-ci Please test

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - fe5af74cf40cbe5c611f2a74e6ef53b8bca04b38

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - fe5af74cf40cbe5c611f2a74e6ef53b8bca04b38

Also updates GenClangType.cpp to be able to map from SIMD types to ExtVectors (the reverse mapping and importer support are not done yet, and will be in a follow-on patch), and updates tests to account for the new spellings.
@stephentyrone
Copy link
Contributor Author

@swift-ci Please test

@stephentyrone
Copy link
Contributor Author

@swift-ci please test source compatibility

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - 8b10812

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - 8b10812

@stephentyrone
Copy link
Contributor Author

@swift-ci Please test

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - 83642ae

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - 83642ae

@stephentyrone
Copy link
Contributor Author

@swift-ci please test

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - 539d6b2

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - 539d6b2

@stephentyrone
Copy link
Contributor Author

@swift-ci please test

@stephentyrone
Copy link
Contributor Author

@swift-ci please test source compatibility

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - 64474a2

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - 64474a2

@stephentyrone
Copy link
Contributor Author

@swift-ci please test

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - 9eea4c4

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - 6c69d94

@stephentyrone
Copy link
Contributor Author

@swift-ci please test OS X

@stephentyrone stephentyrone merged commit fb8b9e1 into swiftlang:master Nov 29, 2018
@gottesmm
Copy link
Contributor

@stephentyrone woot!

@xedin
Copy link
Contributor

xedin commented Nov 30, 2018

I think this should have been reviewed by either me or @rudkx, it looks like after these changes multiple previously resolved crashers are now crashing again (which is very concerning), and performance of expressions with multiple iterators have degraded...

@stephentyrone stephentyrone deleted the simd-2 branch February 8, 2023 00:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants