Jit double zeros #2325
Description
Given sharplab
using System.Buffers;
using System.Runtime.CompilerServices;
public class C
{
public void M()
{
var amb = AsyncValueTaskMethodBuilder<ReadResult>.Create();
}
}
public readonly struct ReadResult
{
internal readonly ReadOnlySequence<byte> _resultBuffer;
internal readonly int _resultFlags;
}The Jit zeros in the preamble with rep stosd and then zeros in the body with vmovdqu
L0000: push rdi
L0001: push rsi
L0002: sub rsp, 0x38
L0006: vzeroupper
L0009: mov rsi, rcx
L000c: lea rdi, [rsp+0x8] ;
L0011: mov ecx, 0xc ;
L0016: xor eax, eax ;
L0018: rep stosd ; clear stack
L001a: mov rcx, rsi
L001d: lea rax, [rsp+0x8] ;
L0022: vxorps xmm0, xmm0, xmm0 ;
L0026: vmovdqu [rax], xmm0 ;
L002a: vmovdqu [rax+0x10], xmm0 ;
L002f: vmovdqu [rax+0x20], xmm0 ; clear stack again
L0034: add rsp, 0x38
L0038: pop rsi
L0039: pop rdi
L003a: retWould it be possible for the Jit to skip the second set of zeroing given its already zeroed the stack at the start?
category:cq
theme:prolog-epilog
skill-level:expert
cost:medium
Activity
[-]Jit over zeros[/-][+]Jit double zeros[/+]AndyAyersMS commentedon Jan 29, 2020
cc @erozenfeld
benaadams commentedon Jan 29, 2020
A more specific/common example is async, for example ReadResults in System.IO.Pipelines; which may be more problematic as currently the C# statemachine generation inserts the saving of parameters before the call to
.Createand assign, which does the second zeroing.erozenfeld commentedon Jan 29, 2020
I'll investigate.
erozenfeld commentedon Feb 3, 2020
#31677 reduces the code for the first example to a single
retinstruction.The second one is harder to deal with. We are very conservative with address-exposed locals and don't try to figure out any liveness information for them. I'm trying to find an inexpensive way to do better at least in some simple cases.
benaadams commentedon Feb 3, 2020
@erozenfeld would this roslyn change help, or not make any difference? dotnet/roslyn#41253 (comment)
Remove popped trees more aggressively. (#31677)
benaadams commentedon Feb 4, 2020
Or would changing it to move all the zeroing above that first assignment to non-zero (which would be a much more complex refactoring in Roslyn)
erozenfeld commentedon Feb 4, 2020
Neither of these two changes will help the jit. It has trouble with double initialization in much simpler cases, e.g., #1007. In your example there are 3 blockers for the jit. Any of them will currently result in double zero-initialization.
Here the explicit initialization happens in the first basic block so maybe we can special-case this somehow.
runtime/src/coreclr/src/jit/lclmorph.cpp
Line 1012 in 2cf4433
runtime/src/coreclr/src/jit/codegencommon.cpp
Lines 4636 to 4640 in 8bd94d1
benaadams commentedon Feb 4, 2020
Am wondering given 3. (prolog initialization); then specialising the first block might be a good route/stopgap?
i.e. if its going to do
rep stosdwhich has a high fixed cost; then its better to use its low variable cost to zero rather than try to anything else, and its probably a common case? e.g. All the examples in #1007 are first block and C# forces explicit initialization; as well as picking up everyasyncStart method which all have this pattern.The first block can assume prolog initialization in certain circumstances e.g. (1.) (3.) above, and only needs to consider itself (e.g. given this type will be zero; has it been overwritten in my block, requiring re-zero); whereas I assume full tracking to eliminate double writes across the entire method would be a lot more involved?
Obviously I'm just armchairing this and don't have any idea of the complexity to do that; so feel free to disregard 😉
erozenfeld commentedon Feb 4, 2020
The jit needs additional analysis in order to remove one of the zero initializations safely.
To remove a zero-initialization of a struct with GC fields in the prolog the jit must make sure that the struct is initialized explicitly before any use (if initlocals is set on the method) and also before any GC-safe point. Note that initlocals is stripped in almost all framework assemblies, so that helps a bit.
To remove an explicit zero-initialization of a struct with GC fields the jit must ensure that the struct is not written-to on any path between the prolog and the explicit initialization. It also needs to check that the explicit initialization is not in a loop.
The jit doesn't currently have that information for address-exposed structs. I don't see a cheap and easy way to get the information. Yes, doing the analysis for explicit initializations in the first basic block may be a good compromise but even that is not straightforward to implement. In your case we are zero-initializing a field of type struct rather than a local struct, which is another complication for the jit. I'll keep exploring the possibilities here.
12 remaining items
erozenfeld commentedon May 23, 2020
@benaadams #36918 addresses
DateTimeParse:ParseExact(ReadOnlySpan`1,ReadOnlySpan`1,DateTimeFormatInfo,int):DateTimecase:
G_M60355_IG01: sub rsp, 184 vzeroupper xor rax, rax - mov qword ptr [rsp+28H], rax + mov qword ptr [rsp+38H], rax vxorps xmm4, xmm4 - vmovdqa xmmword ptr [rsp+30H], xmm4 vmovdqa xmmword ptr [rsp+40H], xmm4 mov rax, -96 vmovdqa xmmword ptr [rsp+rax+B0H], xmm4 vmovdqa xmmword ptr [rsp+rax+C0H], xmm4 vmovdqa xmmword ptr [rsp+rax+D0H], xmm4 add rax, 48 jne SHORT -5 instr mov qword ptr [rsp+B0H], rax G_M60355_IG02: - vxorps xmm0, xmm0 - vmovdqu xmmword ptr [rsp+38H], xmm0 - vmovdqu xmmword ptr [rsp+48H], xmm0 - vmovdqu xmmword ptr [rsp+58H], xmm0 - vmovdqu xmmword ptr [rsp+68H], xmm0 - vmovdqu xmmword ptr [rsp+78H], xmm0 - vmovdqu xmmword ptr [rsp+88H], xmm0 - vmovdqu xmmword ptr [rsp+98H], xmm0 - vmovdqu xmmword ptr [rsp+A8H], xmm0 mov rax, bword ptr [rcx] mov r10d, dword ptr [rcx+8] lea r11, bword ptr [rsp+98H] mov bword ptr [r11], rax mov dword ptr [r11+8], r10d mov dword ptr [rsp+60H], -1 mov dword ptr [rsp+64H], -1 mov dword ptr [rsp+68H], -1 vmovsd xmm0, qword ptr [reloc @RWD00] vmovsd qword ptr [rsp+58H], xmm0 mov dword ptr [rsp+78H], -1 G_M60355_IG03: vmovdqu xmm0, xmmword ptr [rcx] vmovdqu xmmword ptr [rsp+28H], xmm0 ;; bbWeight=1 PerfScore 3.00 G_M60355_IG04: lea rcx, [rsp+38H] mov qword ptr [rsp+20H], rcx lea rcx, bword ptr [rsp+28H] call DateTimeParse:TryParseExact(ReadOnlySpan`1,ReadOnlySpan`1,DateTimeFormatInfo,int,byref):bool test eax, eax je SHORT G_M60355_IG06 mov rax, qword ptr [rsp+90H] G_M60355_IG05: add rsp, 184 ret G_M60355_IG06: lea rcx, [rsp+38H] call DateTimeParse:GetDateTimeParseException(byref):Exception mov rcx, rax call CORINFO_HELP_THROW int3 RWD00 dq BFF0000000000000h -; Total bytes of code 282, prolog size 84, PerfScore 71.27, (MethodHash=de53143c) for method DateTimeParse:ParseExact(ReadOnlySpan`1,ReadOnlySpan`1,DateTimeFormatInfo,int):DateTime +; Total bytes of code 215, prolog size 78, PerfScore 54.23, (MethodHash=de53143c) for method DateTimeParse:ParseExact(ReadOnlySpan`1,ReadOnlySpan`1,DateTimeFormatInfo,int):DateTimeerozenfeld commentedon May 23, 2020
Regarding your second example where we first assign to a gc field and then initialize the struct field: currently my optimization doesn't handle that. It's possible to extend it to handle that case but that would require keeping track of field initializations and may be a little tricky.
However, now the answer to your question
is yes.
If the struct field zero initialization was done before the first assignment, it would be eliminated by the new optimization.
benaadams commentedon May 23, 2020
💜 Thank you; sounds like this issue would be resolved then and the other fixed by a tweak in Roslyn (though that proved a bit hard when I looked; might give it another go though)
benaadams commentedon May 27, 2020
Resolved by #36918
Thank you @erozenfeld 🎉
benaadams commentedon Jun 18, 2020
Looks like the tweak in Rosyn dotnet/roslyn#45262 doesn't enable the zero init optimization
IL code if easier to work with
Output
benaadams commentedon Jun 18, 2020
Will raise new issue; have an easier repo
benaadams commentedon Jun 18, 2020
Opened new issue with a simpler repo #38070