Re-organize intrinsic-test to enable seamless addition of behaviour testing for more architectures
#1758
Conversation
madhav-madhusoodanan
force-pushed
the
restructure-intrinsic-test-crate
branch
2 times, most recently
from
9900e81 to
546554d
Compare
|
The module should be called |
|
@JamieCunliffe feel free to have a look at this since you are the original author of intrinsic-test. This is part of a GSoC project to extend intrinsic-test to other architectures. |
madhav-madhusoodanan
force-pushed
the
restructure-intrinsic-test-crate
branch
6 times, most recently
from
5dce230 to
7ff8497
Compare
|
Are we using the |
madhav-madhusoodanan
force-pushed
the
restructure-intrinsic-test-crate
branch
2 times, most recently
from
a12b4a4 to
ff10311
Compare
|
|
madhav-madhusoodanan
force-pushed
the
restructure-intrinsic-test-crate
branch
9 times, most recently
from
6e8851b to
a2ce02c
Compare
madhav-madhusoodanan
changed the title
intrinsic-test to enable seamless addition of behaviour testing for more architecturesintrinsic-test to enable seamless addition of behaviour testing for more architectures
madhav-madhusoodanan
changed the title
intrinsic-test to enable seamless addition of behaviour testing for more architecturesintrinsic-test to enable seamless addition of behaviour testing for more architectures
…ted for different architectures. Next steps: Move the existing ARM-specific implementation into one that fits well with this trait.
…le_rust and compare_outputs
madhav-madhusoodanan
force-pushed
the
restructure-intrinsic-test-crate
branch
2 times, most recently
from
9dfe061 to
9a221fe
Compare
madhav-madhusoodanan
force-pushed
the
restructure-intrinsic-test-crate
branch
3 times, most recently
from
54434b1 to
e72631d
Compare
madhav-madhusoodanan
force-pushed
the
restructure-intrinsic-test-crate
branch
from
e72631d to
e72b4c1
Compare
| pub fn build_notices(line_prefix: &str) -> String { | ||
| format!( | ||
| "\ | ||
| {line_prefix}This is a transient test file, not intended for distribution. Some aspects of the | ||
| {line_prefix}test are derived from a JSON specification, published under the same license as the | ||
| {line_prefix}`intrinsic-test` crate.\n | ||
| " | ||
| ) | ||
| } |
There was a problem hiding this comment.
Out of curiosity, how important is this notice?
I'm totally in favour of documentation, but seeing as the files that are being generated are purely for testing purposes I thought it'd be best to clarify.
There was a problem hiding this comment.
We would like to keep this notice for Arm intrinsics if possible. For us it's not technically mandatory to include, but it reduces risk for us.
I'm not in a position to speak for what's best for other architectures unfortunately.
|
In the last commit, I migrated from Argument<T, M> to an Argument configuration, since I realized that the Presently, I think I’m at that stage where I'd be better at optimise further when I add in more architectures to Would it be a good strategy on making this PR merge-worthy and then continue making such re-organization efforts in future PRs (as and when I add more architectures)? There was one suggestion made by @adamgemmell about updating the types.rs (now common/intrinsic_helpers.rs) to make it supportive for x86 too. Would it be a good idea if I can work on that in the next PR (in which I’d bring behavioural testing coverage for x86)? |
Updates so far
armmodule, exposing only atestfunction atsrc/main.rsSupportedArchitectureTesttrait that has been implemented forarmand should be implemented for other architectures too. Allows us to expose functionality like building C/Rust test files and comparing outputs.commonmodule for reuse by other architectures too.matchblock for selection of architectures using thetargetCLI variable.Reasoning
Intrinsictype may be specific to architectures, hence the implementation of a trait helps us to abstract the usage of such data types within the architecture-specific moduleNew architecture of
intrinsic-test