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.

Sign up for GitHub

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

Jump to bottom

Unsafe.SizeOf<T>() causes an additional mov over sizeof(T) #55472

Closed
MichalPetryka opened this issue Jul 11, 2021 · 4 comments · Fixed by #63720
Closed

Unsafe.SizeOf<T>() causes an additional mov over sizeof(T) #55472

MichalPetryka opened this issue Jul 11, 2021 · 4 comments · Fixed by #63720
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI tenet-performance Performance related issue
Milestone
Future

Comments

@MichalPetryka
Copy link
Contributor

Description

In some cases, Unsafe.SizeOf<T> produces an additional mov over sizeof(T) (both over not compile time known types).

Configuration

Sharplab Core CLR v5.0.721.25508

Regression?

No idea.

Data

Sharplab
@MichalPetryka MichalPetryka added the tenet-performance Performance related issue label Jul 11, 2021
@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Jul 11, 2021
@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.

@jkotas jkotas added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Jul 11, 2021
@GrabYourPitchforks
Copy link
Member

@jkotas There's an existing proposal to move the Unsafe type down to corelib. Might this issue be resolved as a side effect of that work?

@jkotas
Copy link
Member

jkotas commented Jul 11, 2021

proposal to move the Unsafe type down to corelib.

... and turn it into JIT intrinsic. I agree that turning Unsafe into JIT intrinsic is likely to fix this issue.

@AndyAyersMS
Copy link
Member

Also likely to be fixed if/when we implement forward sub (#6973)

@AndyAyersMS AndyAyersMS removed the untriaged New issue has not been triaged by the area owner label Jul 12, 2021
@AndyAyersMS AndyAyersMS added this to the Future milestone Jul 12, 2021
AndyAyersMS added a commit to AndyAyersMS/runtime that referenced this issue Jan 13, 2022
@AndyAyersMS
JIT: simple forward substitution pass
86992b6 
Extend ref counting done by local morph so that we can determine
single-def single-use locals.

Add a phase that runs just after local morph that will attempt to
forward single-def single-use local defs to uses when they are in
adjacent statements.

Fix or work around issues uncovered elsewhere:
* `gtFoldExprCompare` might fold "identical" volatile subtrees
* `fgGetStubAddrArg` cannot handle complex trees
* some simd/hw operations can lose struct handles
* some calls cannot handle struct local args

Addresses dotnet#6973 and related issues. Still sorting through exactly
which ones are fixed, so list below may need revising.

Fixes dotnet#48605.
Fixes dotnet#51599.
Fixes dotnet#55472.

Improves some but not all cases in dotnet#12280 and dotnet#62064.

Does not fix dotnet#33002, dotnet#47082, or dotnet#63116; these require handling multiple
uses or bypassing statements.
@AndyAyersMS AndyAyersMS mentioned this issue Jan 13, 2022
Merged
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Jan 13, 2022
AndyAyersMS added a commit that referenced this issue Feb 4, 2022
@AndyAyersMS
JIT: simple forward substitution pass (#63720)
99dcf5b 
Extend ref counting done by local morph so that we can determine
single-def single-use locals.

Add a phase that runs just after local morph that will attempt to
forward single-def single-use local defs to uses when they are in
adjacent statements.

Fix or work around issues uncovered elsewhere:
* `gtFoldExprCompare` might fold "identical" volatile subtrees
* `fgGetStubAddrArg` cannot handle complex trees
* some simd/hw operations can lose struct handles
* some calls cannot handle struct local args
* morph expects args not to interfere
* fix arm; don't forward sub no return calls
* update debuginfo test (we may want to revisit this)
* handle subbing past normalize on store assignment
* clean up nullcheck of new helper

Addresses #6973 and related issues. Still sorting through exactly
which ones are fixed, so list below may need revising.

Fixes #48605.
Fixes #51599.
Fixes #55472.

Improves some but not all cases in #12280 and #62064.

Does not fix #33002, #47082, or #63116; these require handling multiple
uses or bypassing statements.
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Feb 4, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Mar 6, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI tenet-performance Performance related issue
Projects
None yet
Milestone
Future
Development

Successfully merging a pull request may close this issue.

JIT: simple forward substitution pass AndyAyersMS/runtime
4 participants
@GrabYourPitchforks @jkotas @AndyAyersMS @MichalPetryka