intrinsic-test: refactor populate_random and related code#2126
Open
davidtwco wants to merge 8 commits into
Open
intrinsic-test: refactor populate_random and related code#2126davidtwco wants to merge 8 commits into
populate_random and related code#2126davidtwco wants to merge 8 commits into
Conversation
The arithmetic for computing the length of the array containing test values is repeated in a handful of locations and doesn't support scalable vectors - this commit introduces a `test_values_array_length` helper to deduplicate this logic.
This function is more complex than it needs to be and `populate_random` is an unintuitive name - it doesn't do anything involving randomness.
Move this function to the `values` module alongside the other functions related to generation of the test value arrays and rename with a similar name.
`PASSES` is a constant accessible to all of the `common` module but is sometimes used directly and sometimes threaded through functions as an argument - this commit eliminates the inconsistency and just uses the constant everywhere.
Given that these "values" are sometimes re-interpreted as negative numbers, in practice they are bit patterns rather than values.
When SVE vectors start being tested, the test vectors will have length `$max_sve_bits / $ty_bit_length`, which is longer than most existing vector types, so more values are useful.
This isn't strictly optimal as it means that the 5/6/7 bit cases are less likely to be tested with values at the edges (e.g. `0x7e` for 7 bits).
This function doesn't really generate an argument at all, it writes a static that is eventually loaded from to become the argument.
Collaborator
|
r? @Amanieu rustbot has assigned @Amanieu. Use Why was this reviewer chosen?The reviewer was selected based on:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Each commit has an explanation of what it is trying to do - this PR largely aims to refactor and tidy-up the
populate_randomfunction and the related code around it to be more understandable. It also means that this logic should support SVE.