[Arm64] Replace pairs of str with stp

[kunalspathak]

Optimize pair of str with stp along with tracking the gc information.

Contributes to #55365 and #77010
Fixes: #35134, #35133 and #81278

[ghost]

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

Issue Details

---

Optimize pair of str with stp along with tracking the gc information.

Contributes to #55365 and #77010
Fixes: #35134, #35133 and #81278

Author:	kunalspathak
Assignees:	-
Labels:	area-CodeGen-coreclr
Milestone:	-

[kunalspathak]

Diffs looks good (as expected):

I will run gcstress job to make sure there are no gc holes.

[kunalspathak]

/azp run runtime-coreclr gcstress0x3-gcstress0xc

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[kunalspathak]

/azp run runtime-coreclr gcstress0x3-gcstress0xc

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[kunalspathak]

gcstress is green with few known failures.

[kunalspathak]

@dotnet/jit-contrib , @BruceForstall

cc: @a74nh @SwapnilGaikwad

[BruceForstall]

Diffs

[BruceForstall]

This diffs look great. Interesting there are some zero-size diffs where we convert str/stp to stp/str.

fwiw, I noticed a case maybe we could do better in libraries.crossgen2 on win-arm64:

16194.dasm - System.Reflection.TypeNameParser:GetType(System.String,bool,bool,System.Reflection.Assembly):System.Type
...
             strb    wzr, [fp, #0x38]
             strb    wzr, [fp, #0x39]
             strb    wzr, [fp, #0x3A]
             strb    wzr, [fp, #0x3B]
             strb    wzr, [fp, #0x3C]

=>

             str     wzr, [fp, #0x38]
             strb    wzr, [fp, #0x3C]

?

[BruceForstall]

Can you show any diffs (including GC diffs) where both addresses are GC lcl vars?

[BruceForstall]

Why is this not a variant or extension of emitIns_S_S_R_R?

In that case, no reg3 is passed because it is determined based on the lclvar. In this case, there are 2 lclvar. Presumably they would both have the same FPbased value, thus determining the same address base register. In fact, you could probably determine / assert the offsets are correct.

[kunalspathak]

I have referred the code from emitIns_R_R_R_I() instead which operates on vector registers as well. For example, it operates on scale of 2, 3 and 4, whereas emitIns_S_S_R_R() operates just on scale == 3. Also, it has special handling for useRegForAdr which I assume is just for the stack stores. And yes, the reg3 is always FP or SP.

[BruceForstall]

I presume the MinOpts TP regression is due to extra checks related to instrDesc size, and possibly due to fewer constants fitting in a small instrDesc.

[kunalspathak]

Can you show any diffs (including GC diffs) where both addresses are GC lcl vars?

Sure, this is one example 4997 in coreclr_test.mch of windows.arm64 where I missed the gc tracking, and fixed it in 92b67de.

Here is the diff. Of course, we need to fix #84504

[kunalspathak]

I presume the MinOpts TP regression is due to extra checks related to instrDesc size, and possibly due to fewer constants fitting in a small instrDesc.

correct.

[kunalspathak]

fwiw, I noticed a case maybe we could do better in libraries.crossgen2 on win-arm64:

That is an interesting pattern coming from accessing 4~5 bool fields that are next to each other.

runtime/src/coreclr/System.Private.CoreLib/src/System/Reflection/TypeNameParser.CoreCLR.cs

Lines 20 to 24 in abaad8f

	private bool _throwOnError;
	private bool _ignoreCase;
	private bool _extensibleParser;
	private bool _requireAssemblyQualifiedName;
	private bool _suppressContextualReflectionContext;

Unfortunately, the popular peephole optimization that we do today just check the last instruction. We could use the infrastructure that we have recently developed to see last N instructions and combine them, but honestly, I don't know if removing last N instructions is well tested and works. I would expect this optimization to happen higher up in morph or something, but should be tracked as a separate issue, although I am not sure how many of such patterns exist.

[BruceForstall]

offs here is the offset within a local var. Since there are 2 lclvars, there should be two offsets: offs1, offs2.

[kunalspathak]

The 2nd offset would be just offs + TARGET_POINTER_SIZE so we don't need to pass it.

[BruceForstall]

That's not correct. The offset is within the local, not from the base register.

[kunalspathak]

@BruceForstall - this should be ready to review again.

[kunalspathak]

Latest diffs:

Seems like some good TPdiffs because of consolidation of ldp and stp instructions: