-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Do not address expose a struct having 1 slot #40957
Conversation
Failure is related to #40885. |
@dotnet/jit-contrib |
The regression you show is a size regression, but an improvement in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@BruceForstall , @AndyAyersMS - Should this go in .NET 5 or wait until we snap? |
I suspect this will not clear the current bar for .Net 5 changes. |
Agreed. It's too late for code quality improvements; we should only consider important bug fixes for 5.0. |
@CarolEidt , I didn't realize that I can diff
|
Could you please help me understand why we need to mark hfa arg as exposed or doNotEnreg in general? |
I'm not entirely sure why HFAs in general were marked that way, but I'm reasonably certain that the single-element HFAs being marked that way was an oversight. For the multi-element HFAs, I presume it was just an excess of caution. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
I noticed the following inefficiency where we always pushing the SIMD_8 or SIMD_16 on stack. After talking to @CarolEidt , it turned out to be a simple fix which gives good gains.
Below is the sample diff of TryFindFirstMatchedLane.
The regression I am seeing inside
System.Runtime.Intrinsics.Vector64
1[Byte][System.Byte]:Equals(System.Runtime.Intrinsics.Vector641[Byte]):bool:this
is because of following diffs. I think we might be able to improve it, but I haven't done much investigation.I haven't done much investigation yet. Probably I will sync up with @CarolEidt .