Dynamic generate hwintrinsic tests#79552
Conversation
ghost
assigned
tannergooding
added
area-System.Runtime.Intrinsics
and removed
area-System.Reflection.Metadata
labels
|
Tagging subscribers to this area: @dotnet/area-system-runtime-intrinsics Issue DetailsAs per the feedback on #76642, this updates the generated HWIntrinsic tests to not be checked in and instead to be generated dynamically as part of the build. CC. @jkotas, @stephentoub
|
| <?xml version="1.0" encoding="utf-8"?> | ||
| <Project> | ||
|
|
||
| <Import Project="$([MSBuild]::GetPathOfFileAbove(Directory.Build.targets, $(MSBuildThisFileDirectory)..))" /> | ||
|
|
||
| <ItemGroup> | ||
| <ProjectReference Include="$(RepoRoot)/src/tests/Common/GenerateHWIntrinsicTests/GenerateHWIntrinsicTests_Arm.csproj" ReferenceOutputAssembly="false" /> | ||
| </ItemGroup> | ||
|
|
||
| <PropertyGroup> | ||
| <GeneratedHWIntrinsicTestDirectory Condition="'$(GeneratedHWIntrinsicTestDirectory)' == ''">$(IntermediateOutputPath)$(MSBuildProjectName)/gen/</GeneratedHWIntrinsicTestDirectory> | ||
| <GeneratedHWIntrinsicTestListFile Condition="'$(GeneratedHWIntrinsicTestListFile)' == ''">$(GeneratedHWIntrinsicTestDirectory)GeneratedHWIntrinsicTestList.txt</GeneratedHWIntrinsicTestListFile> | ||
| </PropertyGroup> | ||
|
|
||
| <Target Name="ExecuteGenerateTestsScript" | ||
| Inputs="$(MSBuildAllProjects);$(RepoRoot)/src/tests/Common/GenerateHWIntrinsicTests/GenerateHWIntrinsicTests_Arm.cs" | ||
| Outputs="$(GeneratedHWIntrinsicTestListFile)" | ||
| Condition="'$(Language)' == 'C#'"> | ||
| <Exec Command="$(DotNetCli) run --project $(RepoRoot)/src/tests/Common/GenerateHWIntrinsicTests/GenerateHWIntrinsicTests_Arm.csproj -c $(Configuration) -- $(MSBuildProjectName) $(MSBuildThisFileDirectory)Shared $(GeneratedHWIntrinsicTestDirectory) $(GeneratedHWIntrinsicTestListFile)" /> | ||
| </Target> | ||
|
|
||
| <Target Name="ReadGeneratedHWIntrinsicTestListFile" | ||
| BeforeTargets="BeforeCompile;CoreCompile" | ||
| DependsOnTargets="ExecuteGenerateTestsScript" | ||
| Returns="$(GeneratedHWIntrinsicTestList)" | ||
| Condition="'$(Language)' == 'C#'"> | ||
| <ReadLinesFromFile File="$(GeneratedHWIntrinsicTestListFile)"> | ||
| <Output TaskParameter="Lines" ItemName="GeneratedHWIntrinsicTestList" /> | ||
| </ReadLinesFromFile> | ||
| <ItemGroup> | ||
| <Compile Include="@(GeneratedHWIntrinsicTestList)" /> | ||
| </ItemGroup> | ||
| </Target> | ||
|
|
||
| </Project> |
There was a problem hiding this comment.
This is the actual interesting "bit" of the change.
Each high level folder (Arm, General, X86) has a targets file that:
- Has a dependency on the relevant test generator to ensure it is built first
- A target that runs the test generator to ensure the necessary files exist, with relevant up to date checks to ensure it doesn't run unnecessarily
- A target that has to always run as part of compilation to read the list of generated tests and include them in the list of files csc should process
There was a problem hiding this comment.
The actual GenerateTests.csx was simply moved to be part of a csproj and to take a couple inputs to help filter the tests.
It's not necessarily the cleanest change, but it is by far one of the simplest. Building a source generator or otherwise refactoring things more would result in significantly more time and churn and would block the AVX-512 work until that was completed.
|
FYI @davidwrighton |
|
Should be ready for review/merging |
|
Ping on review here, would be nice to get this merged in so other PRs can be unblocked |
|
Can't type in the review section because the diff is so large 😄. LGTM, I think this seems like a reasonable solution to the feedback. I do think this should get a review from one of the people invested in that conversation (I'll add them as suggested reviewers). @tannergooding it might be worth merging now if this is blocking other work and having those additional reviews happen post-merge (assuming we can easily revert this if there is any pushback). |
dakersnar
requested review from
AaronRobinsonMSFT,
davidwrighton,
jkotas and
stephentoub
3 participants
As per the feedback on #76642, this updates the generated HWIntrinsic tests to not be checked in and instead to be generated dynamically as part of the build.
This resolves #75791
CC. @jkotas, @stephentoub