Skip to content

[WIP][mono] Add ILLink vector substitutions - #79753

Closed
kotlarmilos wants to merge 1 commit into
dotnet:mainfrom
kotlarmilos:vector-substitutions
Closed

[WIP][mono] Add ILLink vector substitutions#79753
kotlarmilos wants to merge 1 commit into
dotnet:mainfrom
kotlarmilos:vector-substitutions

Conversation

@kotlarmilos

Copy link
Copy Markdown
Member

Add ILLink substitutions to allow Vector classes trimming. Size reduction for iOS sample app should be 8%. Similar is done in #79672.

Draft PR should confirm if the runtime works as expected.

cc @ivanpovazan

@kotlarmiloskotlarmilos self-assigned this Dec 16, 2022
@ghost

Copy link
Copy Markdown

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

@ghostghost added the linkable-framework Issues associated with delivering a linker friendly framework label Dec 16, 2022
@ghost

Copy link
Copy Markdown

Tagging subscribers to 'linkable-framework': @eerhardt, @vitek-karas, @LakshanF, @sbomer, @joperezr
See info in area-owners.md if you want to be subscribed.

Issue Details

Add ILLink substitutions to allow Vector classes trimming. Size reduction for iOS sample app should be 8%. Similar is done in #79672.

Draft PR should confirm if the runtime works as expected.

cc @ivanpovazan

Author:kotlarmilos
Assignees:kotlarmilos
Labels:

linkable-framework

Milestone:-

@kotlarmiloskotlarmilos added NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) NO-REVIEW Experimental/testing PR, do NOT review it labels Dec 16, 2022
@kotlarmiloskotlarmilos changed the title [mono] Add ILLink vector substitutions[WIP][mono] Add ILLink vector substitutionsDec 16, 2022
@SamMonoRT

Copy link
Copy Markdown
Member

/cc @fanyang-mono

@fanyang-mono

Copy link
Copy Markdown
Member

I am trying to understand the impact of this change. According to

<ILLinkSubstitutionsXmls Include="$(ILLinkSharedDirectory)ILLink.Substitutions.NoArmIntrinsics.xml" Condition="'$(SupportsArmIntrinsics)' != 'true'" />

ILLink.Substitutions.NoArmIntrinsics.xml will only be included, when SupportsArmIntrinsics is false. Does the value of SupportsArmIntrinsics equal to false on iOS?

@fanyang-mono

Copy link
Copy Markdown
Member

Also, these files are shared between Mono and CoreCLR. I can imagine changing this file would change the behavior of both runtimes. If we want to tailor Mono runtime behavior but not changing CoreCLR, we probably should create a new file.

@ivanpovazan

Copy link
Copy Markdown
Member

ILLink.Substitutions.NoArmIntrinsics.xml will only be included, when SupportsArmIntrinsics is false. Does the value of SupportsArmIntrinsics equal to false on iOS?

@fanyang-mono I believe when targeting iOS devices most of the time the target architecture is arm64, which means that from:

<SupportsArmIntrinsics Condition="'$(Platform)' == 'arm64'">true</SupportsArmIntrinsics>

the linker would actually include NoX86Intrinsics.xml

@kotlarmilos

Copy link
Copy Markdown
MemberAuthor

Also, these files are shared between Mono and CoreCLR. I can imagine changing this file would change the behavior of both runtimes. If we want to tailor Mono runtime behavior but not changing CoreCLR, we probably should create a new file.

Makes sense.

the linker would actually include NoX86Intrinsics.xml

In both scenarios there should a size reduction as some code will be trimmed.

@kotlarmilos

Copy link
Copy Markdown
MemberAuthor

Finding is that in AOT mode for iOS apps we are not allowed to trim fallback code in vectors as JIT and Interpreter don't support vector intrinsics yet. Once vector intrinsics are supported in JIT and Interpreter we will be allowed to trim the fallback code.

@kotlarmilos
kotlarmilos deleted the vector-substitutions branch December 16, 2022 17:02
@ghostghost locked as resolved and limited conversation to collaborators Jan 15, 2023
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

linkable-frameworkIssues associated with delivering a linker friendly frameworkNO-MERGEThe PR is not ready for merge yet (see discussion for detailed reasons)NO-REVIEWExperimental/testing PR, do NOT review it

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@kotlarmilos@SamMonoRT@fanyang-mono@ivanpovazan