Skip to content

Handle additional multireg args - #43870

Merged
CarolEidt merged 3 commits into
dotnet:masterfrom
CarolEidt:EnregNonHfaMultiReg
Dec 2, 2020
Merged

Handle additional multireg args#43870
CarolEidt merged 3 commits into
dotnet:masterfrom
CarolEidt:EnregNonHfaMultiReg

Conversation

@CarolEidt

Copy link
Copy Markdown
Contributor

No description provided.

@Dotnet-GitSync-BotDotnet-GitSync-Bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Oct 27, 2020
@CarolEidt

Copy link
Copy Markdown
ContributorAuthor

@dotnet/jit-contrib PTAL

@BruceForstallBruceForstall left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM. Any diffs?

@CarolEidt

Copy link
Copy Markdown
ContributorAuthor

Here are the diffs for Arm64 and x64/ux:

ArchOSWhatDeltaMethods ImprovedMethods Regressed
Arm64WindowsCrossgen fx+benchmarks-48672 (-0.04%)27851126
x64LinuxCrossgen fx+benchmarks-80288 (-0.08%)3342739
Arm64WindowsPMI fx+benchmarks-77788 (-0.13%)46671140
x64LinuxPMI fx+benchmarks-109433 (-0.21%)53471101

I've sampled some of the regressions, and here are the 3 sources of regressions I've seen (all of the examples below occur on both Arm64 and x64/ux):

  • Tail calls are rejected if there is a promoted struct parameter

    • SPC.dll System.Decimal:Parse(System.ReadOnlySpan`1[Char],int,System.IFormatProvider):System.Decimal (method hash 0xd422ae6c)
    • SPC.dll System.MemoryExtensions:Trim(System.ReadOnlySpan1[Char],System.ReadOnlySpan1[Char]):System.ReadOnlySpan`1[Char] (method hash 0xff09f8a4)
  • Worse codegen for a block copy from a promoted struct to a field of an object. Although the address is put into a local variable, we lose the ability to reuse the value after it's incremented by the helper. Also, we use CORINFO_HELP_CHECKED_ASSIGN_REF instead of CORINFO_HELP_ASSIGN_BYREF. This happens on both Arm64 and x64/ux:

    • xunit.console.dll <>c:b__6_0(System.Collections.Generic.KeyValuePair`2[[System.String, System.Private.CoreLib, Version=6.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e],[System.String, System.Private.CoreLib, Version=6.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]]):System.Action``1[[System.Xml.Linq.XElement, System.Private.Xml.Linq, Version=6.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51]]:this (method hash 0xff355c80)
  • Plus a few instances where the code gets larger because clone a loop.

@sandreenkosandreenko left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

Comment threadsrc/coreclr/src/jit/lclvars.cpp Outdated
if ((structPromotionInfo.fieldCnt != 2) &&
!((structPromotionInfo.fieldCnt == 1) && varTypeIsSIMD(structPromotionInfo.fields[0].fldType)))
{
JITDUMP("Not promoting multireg struct local V%02u, because lvIsParam is true and #fields != 2\n",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
JITDUMP("Not promoting multireg struct local V%02u, because lvIsParam is true and #fields != 2\n",
JITDUMP("Not promoting multireg struct local V%02u, because lvIsParam is true and #fields != 2 and not a single SIMD?\n",

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Done - with a minor rewording

Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-CodeGen-coreclrCLR 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.

4 participants

@CarolEidt@BruceForstall@sandreenko@Dotnet-GitSync-Bot