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

JIT: Transform single-reg args to FIELD_LIST in physical promotion #111590

Merged

Conversation

jakobbotsch
Copy link
Member

@jakobbotsch jakobbotsch commented Jan 19, 2025

Extend the support for struct arguments in physical promotion to support transforming single-reg structs into a single-field FIELD_LIST node. This allows physically promoted struct arguments passed in a single register to stay in registers.

Morph strips the FIELD_LIST in this particular case to match its standard (type-mismatched) representation for single-reg structs. We could produce this IR directly in physical promotion too, but that hits asserts due to type mismatches. Long term we might consider switching to uniformly use FIELD_LIST for structs to avoid the type mismatches we see today.

This extends #104370 to handle the remaining cases.

Extend the support for struct arguments in physical promotion to support
transforming single-reg structs into a single-field FIELD_LIST node.
This allows physically promoted struct arguments passed in a single
register to stay in registers.

Morph strips the FIELD_LIST in this particular case to match its
standard (type-mismatched) representation for single-reg structs. We
could produce this IR directly in physical promotion too, but that hits
asserts due to type mismatches. Long term we might consider switching to
uniformly use FIELD_LIST for structs to avoid the type mismatches we see
today.
@dotnet-issue-labeler dotnet-issue-labeler bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Jan 19, 2025
Copy link
Contributor

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

@@ -2175,7 +2171,7 @@ bool ReplaceVisitor::ReplaceCallArgWithFieldList(GenTreeCall* call, GenTreeLclVa
{
Replacement* rep = nullptr;
if (agg->OverlappingReplacements(argNode->GetLclOffs() + seg.Offset, seg.Size, &rep, nullptr) &&
rep->NeedsWriteBack)
!rep->NeedsReadBack)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The previous check was conservative. It is possible for both NeedsWriteBack and NeedsReadBack to be false, in which case both the struct base local and replacement local are up to date; in that case we should prefer the value from the replacement local. This is the right check for that.

@jakobbotsch jakobbotsch marked this pull request as ready for review January 23, 2025 22:32
@jakobbotsch
Copy link
Member Author

/azp run runtime-coreclr jitstress, runtime-coreclr libraries-jitstress, runtime-jit-experimental

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@jakobbotsch
Copy link
Member Author

Failures look like #111777 and #111625.

cc @dotnet/jit-contrib PTAL @EgorBo

Diffs, diffs with old promotion disabled. Another step towards making physical promotion able to handle all the same cases as old promotion.

@jakobbotsch jakobbotsch requested a review from EgorBo January 31, 2025 15:11
@jakobbotsch jakobbotsch merged commit 5cc9467 into dotnet:main Feb 1, 2025
143 of 150 checks passed
@jakobbotsch jakobbotsch deleted the physical-promotion-single-reg-struct-args branch February 1, 2025 21:07
grendello added a commit to grendello/runtime that referenced this pull request Feb 3, 2025
* main:
  System.Net.Http.WinHttpHandler.StartRequestAsync assertion failed (dotnet#109799)
  Keep test PDB in helix payload for native AOT (dotnet#111949)
  Build the RID-specific System.IO.Ports packages in the VMR (dotnet#112054)
  Always inline number conversions (dotnet#112061)
  Use Contains{Any} in Regex source generator (dotnet#112065)
  Update dependencies from https://github.com/dotnet/arcade build 20250130.5 (dotnet#112013)
  JIT: Transform single-reg args to FIELD_LIST in physical promotion (dotnet#111590)
  Ensure that math calls into the CRT are tracked as needing vzeroupper (dotnet#112011)
  Use double.ConvertToIntegerNative where safe to do in System.Random (dotnet#112046)
  JIT: Compute `fgCalledCount` after synthesis (dotnet#112041)
  Simplify boolean logic in `TimeZoneInfo` (dotnet#112062)
  JIT: Update type when return temp is freshly created (dotnet#111948)
  Remove unused build controls and simplify DotNetBuild.props (dotnet#111986)
  Fix case-insensitive JSON deserialization of enum member names (dotnet#112028)
  WasmAppBuilder: Remove double computation of a value (dotnet#112047)
  Disable LTCG for brotli and zlibng. (dotnet#111805)
  JIT: Improve x86 unsigned to floating cast codegen (dotnet#111595)
  simplify x86 special intrinsic imports (dotnet#111836)
  JIT: Try to retain entry weight during profile synthesis (dotnet#111971)
  Fix explicit offset of ByRefLike fields. (dotnet#111584)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants