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

[crossgen2] Making sure Vector64/128/256 are not generated unless CoreLib is part of the version bubble #13522

Closed
cshung opened this issue Oct 2, 2019 · 7 comments · Fixed by #40820
Assignees
Milestone

Comments

@cshung
Copy link
Member

cshung commented Oct 2, 2019

No description provided.

@cshung cshung self-assigned this Oct 2, 2019
@4creators
Copy link
Contributor

Is it possible to provide a description of the problem and software architecture associated with it? It would make the issue understandable to all coreclr contributors.

@cshung
Copy link
Member Author

cshung commented Oct 3, 2019

Is it possible to provide a description of the problem and software architecture associated with it? It would make the issue understandable to all coreclr contributors.

This issue is created to track the work required to address one of these work items mentioned as a comment in my pull request.

dotnet/coreclr#26772 (review)

The pull request was created as a port of these 3 pull requests (#24689, dotnet/coreclr#24917, dotnet/coreclr#25365). As I understand it, the original pull request allow crossgen to use hardware intrinsics. The port was intended to allow crossgen2 to also use hardware intrinsics.

To be brutally honest, I don't actually understand what the problem is, at least not in a clearly reproducible manner. The comment must have written to make sure certain scenarios will not fail. I have yet to figure out what those scenarios are.

@MichalStrehovsky who authored the original pull requests and the original comment should be able to explain in detail what the problem is.

@MichalStrehovsky
Copy link
Member

To be brutally honest, I don't actually understand what the problem is, at least not in a clearly reproducible manner

@cshung does this comment help? https://github.com/dotnet/coreclr/pull/24689/files#diff-f2c1beb35334bda2c2e3c3cbbc0ccd54R9559-R9566

I wasn't part of the discussion that lead to the initial disabling, but reading through the linked issue gives a good summary of why we need to avoid AOT compiling them for now (until the issue is resolved) and why it would be okay to let precompilation happen in CoreLib (or in things that version with CoreLib).

@cshung cshung removed their assignment Jan 8, 2020
@msftgits msftgits transferred this issue from dotnet/coreclr Jan 31, 2020
@msftgits msftgits added this to the 5.0 milestone Jan 31, 2020
@mangod9
Copy link
Member

mangod9 commented Aug 12, 2020

@janvorli is there anything actionable here?

@janvorli
Copy link
Member

I believe @davidwrighton has fixed that in one of his changes a long time ago, but I am not 100% sure.

@davidwrighton
Copy link
Member

I'll look into this in depth tomorrow.

@davidwrighton
Copy link
Member

This didn't get implemented, but I've just put together a PR that identifies where the ABI may be unstable and surgically disables code gen for those cases. It preserves the ability to use Vector intrinsics even in the presence of unstable abi.

@ghost ghost locked as resolved and limited conversation to collaborators Dec 12, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants