[JIT] x86/x64 - improved localloc codegen in some cases

[TIHan]

Description

Should resolve: #6546

@BruceForstall and I went through this, and it turns out that it was actually quite simple.

The x86/x64 emitter keeps track of the stack whenever it sees the pattern sub esp, but there are cases where we do not want to do this. To prevent it from keeping track, it would move esp into a temporary register, do sub with that register, and then move it back to esp. This causes two extra instructions to be emitted for x86 under this scenario.

The change adds a new instruction, INS_sub_hide, that does the same thing as sub, but will tell the emitter not to track the stack. We already have other instructions to do this exact thing already with push and pop:

// Does not affect the stack tracking in the emitter
INST5(push_hide,        "push",             IUM_RD, 0x0030FE,     0x000068,     BAD_CODE,     BAD_CODE,     0x000050,    INS_FLAGS_None )
INST5(pop_hide,         "pop",              IUM_WR, 0x00008E,     BAD_CODE,     BAD_CODE,     BAD_CODE,     0x000058,    INS_FLAGS_None )

Example diff:

Acceptance Criteria

• Remove regTmp from genStackPointerConstantAdjustment and possibly further up.

[ghost]

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

Issue Details

---

Description
@BruceForstall and I went through this, and it turns out that it was actually quite simple.

The x86 emitter keeps track of the stack whenever it sees the pattern sub esp, but there are cases where we do not want to do this. To prevent it from keeping track, it would move esp into a temporary register, do sub with that register, and then move it back to esp. This causes two extra instructions to be emitted for x86 under this scenario.

The change adds a new instruction, INS_sub_hide, that does the same thing as sub, but will tell the emitter not to track the stack. We already have other instructions to do this exact thing already with push and pop:

// Does not affect the stack tracking in the emitter
INST5(push_hide,        "push",             IUM_RD, 0x0030FE,     0x000068,     BAD_CODE,     BAD_CODE,     0x000050,    INS_FLAGS_None )
INST5(pop_hide,         "pop",              IUM_WR, 0x00008E,     BAD_CODE,     BAD_CODE,     BAD_CODE,     0x000058,    INS_FLAGS_None )

Acceptance Criteria

• RemoveregTmp from genStackPointerConstantAdjustment and possibly further up.

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

[TIHan]

@dotnet/jit-contrib This is ready assuming CI succeeds. cc @BruceForstall

[kunalspathak]

@dotnet/jit-contrib This is ready assuming CI succeeds. cc @BruceForstall

Could you paste sample diffs?

[TIHan]

Almost all diffs look like this:

Was actually surprised, I didn't expect to get x64 diffs but I did find another part where we did the mov, sub, mov in a potential x64 path, so I replaced it using sub_hide.

[TIHan]

/azp run runtime-coreclr outerloop

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[TIHan]

@BruceForstall Outerloop did pass, apart from the formatting failures.

[TIHan]

@BruceForstall I've made changes to BuildLclHeap in LSRA - also added asserts to the temp register count in codegen to verify.
I ran SuperPMI replay locally for both x86/x64 and they succeeded.

[BruceForstall]

Couple more comments

[TIHan]

@dotnet/jit-contrib This is ready again.

[BruceForstall]

Diffs