Skip to content
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.

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

Reduce compile-time issues #376

Closed
wants to merge 5 commits into from

Conversation

Projects
None yet
2 participants
@gnzlbg
Copy link
Collaborator

gnzlbg commented Mar 16, 2018

Adds the following --cfg attributes:

  • test_intr: enables the core::arch tests
  • test_v{16,32,...,512}: enables the portable vector tests for the different sizes

First we continue to run the stdsimd tests as usual, but now these include only the doc tests and the detect (run-time feature detection) tests, as well as building examples and running the tests in the /tests folders.

Then we use cargo rustc ... -- --cfg test_{sub_test_name} to build the tests for each of the sub-tests above, and finally we just loop over all the binaries running all these tests.

This makes rustc much happier, because instead of compiling all tests at once, it compiles them in batches. The only issue is that we have to recompile coresimd for each test_{...} case above, but at least it is only coresimd - the dev dependencies are compiled only once.

@gnzlbg gnzlbg requested a review from alexcrichton Mar 16, 2018

gnzlbg added some commits Mar 16, 2018

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Mar 16, 2018

Nice! I'm personally always a fan though of staying as close to cargo test as possible, so I wonder if we could perhaps split crates out? Testing the coresimd intrinsics is already a huge pain because of recompilation times when modifing, but if the intrinsics were tested in a separate crate I don't think that'd be an issue, right? We could perhaps have a test suite per architecture?

I think that may bring the compile times down to a reasonable level for the portable intrinsics to be unconditionally compiled when testing coresimd?

@gnzlbg

This comment has been minimized.

Copy link
Collaborator Author

gnzlbg commented Mar 16, 2018

Testing the coresimd intrinsics is already a huge pain because of recompilation times when modifing, but if the intrinsics were tested in a separate crate I don't think that'd be an issue, right? We could perhaps have a test suite per architecture?

The problems aren't really the intrinsics, the problem is the tests for the portable vector types.

Right now all APIs are implemented via macros for all types, and these macros also add the tests for each type. This way when we add a new API or a new vector type, tests are automatically added for all cases, and this catches a lot of errors.

We could just put the tests of the portable vector types into a coresimd/tests/ subdirectory, with one vector size per file, but then we need to manually add the tests for each vector type there. If we could automate that somehow, then that would be great. Otherwise I would prefer to keep the tests together until the portable vector types APIs stop changing much.

@gnzlbg

This comment has been minimized.

Copy link
Collaborator Author

gnzlbg commented Mar 16, 2018

For example, another approach could be to link the coresimd/src/ppsv/v{...}.rs files to coresimd/test/v{...}.rs and somehow leave only the tests in there, but i couldn't make this work.

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Mar 16, 2018

Hm we don't expect the number of portable vectors to be changing much though, right? In that sense if an API is added/changed the test needs to be updated/added anyway, so would that work alright to split those out?

@gnzlbg gnzlbg closed this Mar 16, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.