Skip to content

Add more DataFlow tests from linker to NativeAOT - #72777

Merged
vitek-karas merged 7 commits into
dotnet:mainfrom
vitek-karas:AotTests2
Aug 1, 2022
Merged

Add more DataFlow tests from linker to NativeAOT#72777
vitek-karas merged 7 commits into
dotnet:mainfrom
vitek-karas:AotTests2

Conversation

@vitek-karas

@vitek-karasvitek-karas commented Jul 25, 2022

Copy link
Copy Markdown
Member

This adds several dataflow tests from linker - only those which are passing with the fixes in this change. More will be added later.

Fixes:

  • Correctly handle Requires attributes in generic types
  • Recognize ref Type as interesting for data flow
  • Fix generic parameter propagation for compiler generated types
  • Treat multi-dimensional arrays as "Unknown" values (to match linker behavior)

@vitek-karas
Add more DataFlow tests from linker to NativeAOT
a44d9a2
This adds several dataflow tests from linker - only those which are passing with the fixes in this change. More will be added later.
Fixes:
- Correctly handle Requires attributes in generic types
- Recognize `ref Type` as interesting for data flow
- Fix generic parameter propagation for compiler generated types
- Treat multi-dimensional arrays as "Unknown" values (to match linker behavior)
@vitek-karasvitek-karas self-assigned this Jul 25, 2022
@vitek-karas

vitek-karas commented Jul 25, 2022

Copy link
Copy Markdown
MemberAuthor

The changes to the tests are ported back to linker in dotnet/linker#2912.

@vitek-karasvitek-karas mentioned this pull request Jul 25, 2022
Merged
tlakollo
tlakollo reviewed Jul 25, 2022
View reviewed changes

@tlakollotlakollo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we sure about having all these tests whose main focus is to verify if things are kept? I think once we start to validate the kept attribute now we will have to do it 100% right or we will break CI

Comment threadsrc/coreclr/tools/aot/Mono.Linker.Tests.Cases/DataFlow/AttributePropertyDataflow.cs Outdated
Comment threadsrc/coreclr/tools/aot/Mono.Linker.Tests.Cases/DataFlow/MethodByRefReturnDataFlow.cs Outdated
Comment threadsrc/coreclr/tools/aot/Mono.Linker.Tests.Cases/DataFlow/UnresolvedMembers.cs
@vitek-karas
Merge remote-tracking branch 'dotnet/main' into AotTests2
86f5b9f
@vitek-karas
Improve attribute data flow by providing the correct MessageOrigin. T…
c1ca4bc
…his changes MetadataManager interface, but it should not be a problem.
This fixes the attribute test failures.
The CompilerGeneratedCodeDataflow test was failing because AOT didn't scan the body of the MoveNext method in the state machine for iterators. This is because the test actually never references IEnumerator<>.MoveNext and so the interface method and all of its implementations are removed. Fixed by making an explicit reference to it at least once.
@vitek-karas

vitek-karas commented Jul 25, 2022

Copy link
Copy Markdown
MemberAuthor

Please see c1ca4bc for fixes to all CI failures.

@vitek-karas
Fix a bug in ref-return-value data flow
4025601
This was missed during porting of the changes from linker.
More cleanup of tests.
@vitek-karas
Fix attribute parent calculation by forcing it to be passed from the …
a8646f6
…caller
MichalStrehovsky
MichalStrehovsky approved these changes Aug 1, 2022
View reviewed changes

@MichalStrehovskyMichalStrehovsky left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@vitek-karas
vitek-karas merged commit 05c6f3d into dotnet:mainAug 1, 2022
@vitek-karas
vitek-karas deleted the AotTests2 branch August 1, 2022 08:26
@ghostghost locked as resolved and limited conversation to collaborators Aug 31, 2022
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@MichalStrehovskyMichalStrehovskyMichalStrehovsky approved these changes
+1 more reviewer
@tlakollotlakollotlakollo left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@vitek-karasvitek-karas

Labels

area-NativeAOT-coreclr

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@vitek-karas@MichalStrehovsky@tlakollo