Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Some hardware intrinsic ILLInk.Substitution entries may be unnecessary #83164

Closed
tannergooding opened this issue Mar 8, 2023 · 4 comments
Closed
Labels
area-Tools-ILLink .NET linker development as well as trimming analyzers

Comments

@tannergooding
Copy link
Member

As per #83040 (comment), some of the ILLink.Substitution entries may be unnecessary since corelib already compiles in a different copy of the ISAs (*.PlatformNotSupported.cs) which return constant false.

We should double check that removing these are ok and make the files shorter if possible.

-- Noting the Vector64/128/256/512 entries are needed since they do not return constant false and are instead recursive by default.

@dotnet-issue-labeler
Copy link

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

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Mar 8, 2023
@jkotas
Copy link
Member

jkotas commented Mar 9, 2023

copy of the ISAs (*.PlatformNotSupported.cs) which return constant false.

These methods are marked with [Intrinsic] attribute. I believe that it is make AltJit work, among other things.

Linker does not look at bodies of methods marked `[Intrinsic]. The IL of these methods often does not match what the method actually does. The substitution is required to tell the linker what the method actually does.

@jkotas jkotas added the area-Tools-ILLink .NET linker development as well as trimming analyzers label Mar 9, 2023
@ghost
Copy link

ghost commented Mar 9, 2023

Tagging subscribers to this area: @agocke, @sbomer, @vitek-karas
See info in area-owners.md if you want to be subscribed.

Issue Details

As per #83040 (comment), some of the ILLink.Substitution entries may be unnecessary since corelib already compiles in a different copy of the ISAs (*.PlatformNotSupported.cs) which return constant false.

We should double check that removing these are ok and make the files shorter if possible.

-- Noting the Vector64/128/256/512 entries are needed since they do not return constant false and are instead recursive by default.

Author: tannergooding
Assignees: -
Labels:

untriaged, area-Tools-ILLink

Milestone: -

@tannergooding
Copy link
Member Author

Thanks for the info! Will close this as unnecessary then.

@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Mar 9, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Apr 8, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Tools-ILLink .NET linker development as well as trimming analyzers
Projects
None yet
Development

No branches or pull requests

2 participants