Skip to content

Optimize stackalloc zeroing via BLK - #83255

Merged
EgorBo merged 19 commits into
dotnet:mainfrom
EgorBo:stackalloc-zero-simd
Apr 4, 2023
Merged

Optimize stackalloc zeroing via BLK#83255
EgorBo merged 19 commits into
dotnet:mainfrom
EgorBo:stackalloc-zero-simd

Conversation

@EgorBo

@EgorBoEgorBo commented Mar 10, 2023

Copy link
Copy Markdown
Member

Closes#63500

Let's insert GT_BLK after GT_HEAPLCL to rely on the former to perform zeroing.

Codegen example:

voidTest(){varp=stackallocbyte[250];Consume(p);}

Main:

; Method Program:Test():thisG_M000_IG01: 55pushrbp 4883EC30 subrsp,48 488D6C2420 learbp,[rsp+20H] 48B8218F332784F40000 movrax,0xF48427338F21 ;; GS-cookie48894508mov qword ptr [rbp+08H],raxG_M000_IG02:  4883C420 addrsp,32 B910000000 movecx,16G_M000_IG03:  6A00 push0 6A00 push0 48FFC9 decrcx 75F7 jne SHORT G_M000_IG03 ;; slow loop (zeroing 16 bytes at once) 4883EC20 subrsp,32 488D4C2420 learcx,[rsp+20H] FF1577AA2800 call[Program:Consume(ulong)] 48B9218F332784F40000 movrcx,0xF48427338F21 48394D08 cmp qword ptr [rbp+08H],rcx7405je SHORT G_M000_IG04 E852F6BB5F call CORINFO_HELP_FAIL_FASTG_M000_IG04: 90nopG_M000_IG05:  488D6510 learsp,[rbp+10H] 5D poprbp C3 ret; Total bytes of code: 85

PR:

; Method Progr:Test():thisG_M435_IG01: 55pushrbp 4883EC30 subrsp,48 C5F877 vzeroupper 488D6C2420 learbp,[rsp+20H] 48B878563412F0DEBC9A movrax,0x9ABCDEF01234567848894508mov qword ptr [rbp+08H],raxG_M435_IG02: 852424test dword ptr [rsp],esp 4881EC00010000 subrsp,256 488D542420 leardx,[rsp+20H] C5FC57C0 vxorps ymm0,ymm0 C5FE7F02 vmovdqu ymmword ptr[rdx],ymm0 C5FE7F4220 vmovdqu ymmword ptr[rdx+20H],ymm0 C5FE7F4240 vmovdqu ymmword ptr[rdx+40H],ymm0 C5FE7F4260 vmovdqu ymmword ptr[rdx+60H],ymm0 C5FE7F8280000000 vmovdqu ymmword ptr[rdx+80H],ymm0 C5FE7F82A0000000 vmovdqu ymmword ptr[rdx+A0H],ymm0 C5FE7F82C0000000 vmovdqu ymmword ptr[rdx+C0H],ymm0 C5FE7F82E0000000 vmovdqu ymmword ptr[rdx+E0H],ymm0 FF15F9926000 call[Progr:Consume(ulong):this] 48B978563412F0DEBC9A movrcx,0x9ABCDEF012345678 48394D08 cmp qword ptr [rbp+08H],rcx7405je SHORT G_M435_IG03 E85472505F call CORINFO_HELP_FAIL_FASTG_M435_IG03: 90nopG_M435_IG04:  488D6510 learsp,[rbp+10H] 5D poprbp C3 ret; Total bytes of code: 131

For large constants, this PR switches to call memset while current Main's impl will still be doing that loop of double-push.

Benchmark

BenchmarkSwitcher.FromAssembly(typeof(StackallocTests).Assembly).Run(args);[CsvExporter]publicunsafeclassStackallocBenchmarks{[Benchmark]publicvoidStackalloc8(){byte*ptr=stackallocbyte[8];Consume(ptr);}[Benchmark]publicvoidStackalloc16(){byte*ptr=stackallocbyte[16];Consume(ptr);}[Benchmark]publicvoidStackalloc20(){byte*ptr=stackallocbyte[20];Consume(ptr);}[Benchmark]publicvoidStackalloc32(){byte*ptr=stackallocbyte[32];Consume(ptr);}[Benchmark]publicvoidStackalloc36(){byte*ptr=stackallocbyte[36];Consume(ptr);}[Benchmark]publicvoidStackalloc40(){byte*ptr=stackallocbyte[40];Consume(ptr);}[Benchmark]publicvoidStackalloc50(){byte*ptr=stackallocbyte[50];Consume(ptr);}[Benchmark]publicvoidStackalloc64(){byte*ptr=stackallocbyte[64];Consume(ptr);}[Benchmark]publicvoidStackalloc65(){byte*ptr=stackallocbyte[65];Consume(ptr);}[Benchmark]publicvoidStackalloc80(){byte*ptr=stackallocbyte[80];Consume(ptr);}[Benchmark]publicvoidStackalloc100(){byte*ptr=stackallocbyte[100];Consume(ptr);}[Benchmark]publicvoidStackalloc110(){byte*ptr=stackallocbyte[110];Consume(ptr);}[Benchmark]publicvoidStackalloc128(){byte*ptr=stackallocbyte[128];Consume(ptr);}[Benchmark]publicvoidStackalloc129(){byte*ptr=stackallocbyte[129];Consume(ptr);}[Benchmark]publicvoidStackalloc150(){byte*ptr=stackallocbyte[150];Consume(ptr);}[Benchmark]publicvoidStackalloc180(){byte*ptr=stackallocbyte[180];Consume(ptr);}[Benchmark]publicvoidStackalloc220(){byte*ptr=stackallocbyte[220];Consume(ptr);}[Benchmark]publicvoidStackalloc256(){byte*ptr=stackallocbyte[256];Consume(ptr);}[Benchmark]publicvoidStackalloc257(){byte*ptr=stackallocbyte[257];Consume(ptr);}[Benchmark]publicvoidStackalloc300(){byte*ptr=stackallocbyte[300];Consume(ptr);}[Benchmark]publicvoidStackalloc400(){byte*ptr=stackallocbyte[400];Consume(ptr);}[Benchmark]publicvoidStackalloc500(){byte*ptr=stackallocbyte[500];Consume(ptr);}[Benchmark]publicvoidStackalloc1024(){byte*ptr=stackallocbyte[1024];Consume(ptr);}[Benchmark]publicvoidStackalloc4096(){byte*ptr=stackallocbyte[4096];Consume(ptr);}[MethodImpl(MethodImplOptions.NoInlining)]staticvoidConsume(byte*ptr){}}

Core i7 8700K

image

Ryzen 7950X

image

NOTE: 32 bytes and lower are handled separately so there are no differences for them.

@ghostghost assigned EgorBoMar 10, 2023
@ghostghost added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Mar 10, 2023
@ghost

Copy link
Copy Markdown

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

Issue Details

Let's see if this works - I just insert GT_BLK (basically, Unsafe.InitMemoryUnaligned) after CEE_LOCALLOC in importer to rely on that for zeroing. GT_BLK has its own logic to unroll/emit MEMSET.

[Benchmark]publicvoidStackalloc50(){byte*ptr=stackallocbyte[40];Consume(ptr);}[Benchmark]publicvoidStackalloc50(){byte*ptr=stackallocbyte[50];Consume(ptr);}[Benchmark]publicvoidStackalloc50(){byte*ptr=stackallocbyte[64];Consume(ptr);}[Benchmark]publicvoidStackalloc50(){byte*ptr=stackallocbyte[100];Consume(ptr);}[Benchmark]publicvoidStackalloc50(){byte*ptr=stackallocbyte[128];Consume(ptr);}[Benchmark]publicvoidStackalloc50(){byte*ptr=stackallocbyte[150];Consume(ptr);}[Benchmark]publicvoidStackalloc50(){byte*ptr=stackallocbyte[256];Consume(ptr);}[Benchmark]publicvoidStackalloc50(){byte*ptr=stackallocbyte[512];Consume(ptr);}[Benchmark]publicvoidStackalloc50(){byte*ptr=stackallocbyte[1024];Consume(ptr);}[Benchmark]publicvoidStackalloc50(){byte*ptr=stackallocbyte[4096];Consume(ptr);}[Benchmark]publicvoidStackalloc50(){byte*ptr=stackallocbyte[8192];Consume(ptr);}[MethodImpl(MethodImplOptions.NoInlining)]staticvoidConsume(byte*ptr){}
| Method | Toolchain | Mean | Ratio |
|--------------- |-------------------------- |-----------:|------:|
| Stackalloc40 | \Core_Root\corerun.exe | 1.493 ns | 1.00 |
| Stackalloc40 | \Core_Root_PR\corerun.exe | 1.495 ns | 1.00 |
| | | | |
| Stackalloc50 | \Core_Root\corerun.exe | 2.362 ns | 1.00 |
| Stackalloc50 | \Core_Root_PR\corerun.exe | 1.321 ns | 0.56 |
| | | | |
| Stackalloc64 | \Core_Root\corerun.exe | 2.354 ns | 1.00 |
| Stackalloc64 | \Core_Root_PR\corerun.exe | 1.326 ns | 0.56 |
| | | | |
| Stackalloc100 | \Core_Root\corerun.exe | 3.020 ns | 1.00 |
| Stackalloc100 | \Core_Root_PR\corerun.exe | 1.316 ns | 0.44 |
| | | | |
| Stackalloc128 | \Core_Root\corerun.exe | 3.230 ns | 1.00 |
| Stackalloc128 | \Core_Root_PR\corerun.exe | 1.508 ns | 0.47 |
| | | | |
| Stackalloc150 | \Core_Root\corerun.exe | 3.792 ns | 1.00 |
| Stackalloc150 | \Core_Root_PR\corerun.exe | 4.536 ns | 1.20 |
| | | | |
| Stackalloc256 | \Core_Root\corerun.exe | 6.295 ns | 1.00 |
| Stackalloc256 | \Core_Root_PR\corerun.exe | 4.534 ns | 0.72 |
| | | | |
| Stackalloc512 | \Core_Root\corerun.exe | 13.617 ns | 1.00 |
| Stackalloc512 | \Core_Root_PR\corerun.exe | 4.760 ns | 0.35 |
| | | | |
| Stackalloc1024 | \Core_Root\corerun.exe | 26.741 ns | 1.00 |
| Stackalloc1024 | \Core_Root_PR\corerun.exe | 6.725 ns | 0.25 |
| | | | |
| Stackalloc4096 | \Core_Root\corerun.exe | 109.259 ns | 1.00 |
| Stackalloc4096 | \Core_Root_PR\corerun.exe | 30.131 ns | 0.28 |
| | | | |
| Stackalloc8192 | \Core_Root\corerun.exe | 217.935 ns | 1.00 |
| Stackalloc8192 | \Core_Root_PR\corerun.exe | 57.974 ns | 0.27 |
Author:EgorBo
Assignees:EgorBo
Labels:

area-CodeGen-coreclr

Milestone:-

@stephentoub

Copy link
Copy Markdown
Member

after 128 PR switches to MEMSET

For comparison, what does the graph look like if you don't do that?

@EgorBo

EgorBo commented Mar 10, 2023

Copy link
Copy Markdown
MemberAuthor

after 128 PR switches to MEMSET

For comparison, what does the graph look like if you don't do that?

It looks like we might want to revise our heuristics, e.g. here what Clang/LLVM does:
For a generic CPU with AVX it unrolls zeroing up to 256 bytes, e.g.: https://godbolt.org/z/59Mdodc7T (NOTE that I'm using -Os that stands for "Optimize but keep binary size sane")

For Zen4 (AMD 7xxx) it unrolls up to 512 bytes (AVX512): https://godbolt.org/z/PxvoE4P9r

For a generic CPU without AVX it unrolls up to 128 bytes: https://godbolt.org/z/b4vd13PMz

Our threshold is hard-coded to 128. (and 256 for ARM64)

@EgorBo

Copy link
Copy Markdown
MemberAuthor

NOTE: afair, some (most?) libs in BCL use skiplocalsinit globally so they won't benefit from this change. But to properly test it I need to remove that flag.

@stephentoub

stephentoub commented Mar 10, 2023

Copy link
Copy Markdown
Member

some (most?) libs in BCL use skiplocalsinit globally

Yes, everything in the shared framework:

<SkipLocalsInitCondition="'$(SkipLocalsInit)' == '' and '$(MSBuildProjectExtension)' == '.csproj' and '$(IsNETCoreAppSrc)' == 'true' and '$(TargetFrameworkIdentifier)' == '.NETCoreApp'">true</SkipLocalsInit>

@EgorBo

EgorBo commented Mar 10, 2023

Copy link
Copy Markdown
MemberAuthor

cc @anthonycanino (in case if you're interested adjusting the BLK unroll heuristic for avx-512)

@EgorBo
EgorBo marked this pull request as ready for review March 14, 2023 09:57
@EgorBo

Copy link
Copy Markdown
MemberAuthor

after 128 PR switches to MEMSET

For comparison, what does the graph look like if you don't do that?

Updated. Fixed via #83274

@EgorBo
EgorBo marked this pull request as draft March 14, 2023 13:02
@EgorBo
EgorBoforce-pushed the stackalloc-zero-simd branch from 8c1d241 to 167ce5aCompareMarch 15, 2023 21:32
@EgorBo
EgorBoforce-pushed the stackalloc-zero-simd branch from a4a84ab to 588b964CompareMarch 16, 2023 01:08
Comment threadsrc/coreclr/jit/codegenxarch.cpp Outdated
@runfoapprunfoappBot mentioned this pull request Mar 16, 2023
@benaadams

benaadams commented Mar 17, 2023

Copy link
Copy Markdown
Member

Is stackalloc different to locals? As .NET 7 will partially unroll and loop the local zeroing (though currently only uses xmm)

 vxorps xmm4,xmm4movrax,-0x2340 vmovdqa xmmword ptr [rbp+rax-60H],xmm4 vmovdqa xmmword ptr [rbp+rax-50H],xmm4 vmovdqa xmmword ptr [rbp+rax-40H],xmm4addrax,48jne SHORT -5 instr

Rather than this weird thing

G_M000_IG03: push0push0decrcxjne SHORT G_M000_IG03 ;; slow loop (zeroing 16 bytes at once)

@benaadams

Copy link
Copy Markdown
Member

i.e. should stackalloc be part of locals?

@EgorBo

EgorBo commented Mar 17, 2023

Copy link
Copy Markdown
MemberAuthor

i.e. should stackalloc be part of locals?

Good question. We do that if stackalloc is smaller than 32 bytes. The problem with that that we'll have to zero it all the time no matter if we need it or not (since it's in the prologue). E.g. let me raise that limit to 128 bytes and check codegen for this:

voidTest(boolcond){if(cond){// rarely taken conditionvarp=stackallocbyte[128];Consume(p);}else{Console.WriteLine();}}

Codegen:

; Method Program:Test(bool):thisG_M34929_IG01: ;; offset=0000H 4881ECA8000000 subrsp,168 C5D857E4 vxorps xmm4,xmm4 C5F97F642420 vmovdqa xmmword ptr [rsp+20H],xmm4 C5F97F642430 vmovdqa xmmword ptr [rsp+30H],xmm4 48B8A0FFFFFFFFFFFFFF movrax,-96 C5F97FA404A0000000 vmovdqa xmmword ptr [rsp+rax+A0H],xmm4 C5F97FA404B0000000 vmovdqa xmmword ptr [rsp+rax+B0H],xmm4 C5F97FA404C0000000 vmovdqa xmmword ptr [rsp+rax+C0H],xmm4 4883C030 addrax,48 75DF jne SHORT -5 instr 48B878563412F0DEBC9A movrax,0x9ABCDEF012345678 48898424A0000000 mov qword ptr [rsp+A0H],rax ;; size=84 bbWeight=1 PerfScore 13.33G_M34929_IG02: ;; offset=0054H 84D2 testdl,dl 742D je SHORT G_M34929_IG06 ;; size=4 bbWeight=1 PerfScore 1.25G_M34929_IG03: ;; offset=0058H 488D4C2420 learcx,[rsp+20H] FF15150A7100 call[Program:Consume(ulong)] 48B978563412F0DEBC9A movrcx,0x9ABCDEF012345678 48398C24A0000000 cmp qword ptr [rsp+A0H],rcx7405je SHORT G_M34929_IG04 E824CA4B5F call CORINFO_HELP_FAIL_FAST ;; size=36 bbWeight=0.50 PerfScore 3.88G_M34929_IG04: ;; offset=007CH90nop ;; size=1 bbWeight=0.50 PerfScore 0.12G_M34929_IG05: ;; offset=007DH 4881C4A8000000 addrsp,168 C3 ret ;; size=8 bbWeight=0.50 PerfScore 0.62G_M34929_IG06: ;; offset=0085H FF152DA69000 call[System.Console:WriteLine()] 48B978563412F0DEBC9A movrcx,0x9ABCDEF012345678 48398C24A0000000 cmp qword ptr [rsp+A0H],rcx7405je SHORT G_M34929_IG07 E8FCC94B5F call CORINFO_HELP_FAIL_FAST ;; size=31 bbWeight=0.50 PerfScore 3.62G_M34929_IG07: ;; offset=00A4H90nop ;; size=1 bbWeight=0.50 PerfScore 0.12G_M34929_IG08: ;; offset=00A5H 4881C4A8000000 addrsp,168 C3 ret ;; size=8 bbWeight=0.50 PerfScore 0.62; Total bytes of code: 173

Also, here we don't do stack probing.
Probably, I should decrease that threshold from 32 to some single-instruction sized

@EgorBo
EgorBo marked this pull request as ready for review March 18, 2023 00:12
@EgorBo

Copy link
Copy Markdown
MemberAuthor

@jakobbotsch@BruceForstall @dotnet/jit-contrib PTAL

I inject a BLK node in Lower for all stackalloc nodes (GT_LCLHEAP) with uses. Will enable arm64 separately, its default impl is good but it doesn't switch to memset call for large buffers so still will benefit from BLK too.

Comment threadsrc/coreclr/jit/importer.cpp Outdated
Comment threadsrc/coreclr/jit/lower.cpp Outdated
Comment threadsrc/coreclr/jit/lower.cpp Outdated
EgorBoand others added 5 commits March 31, 2023 19:41
Co-authored-by: SingleAccretion <62474226+SingleAccretion@users.noreply.github.com>
Comment threadsrc/coreclr/jit/lower.cpp Outdated
@EgorBo
EgorBo merged commit e13f0dc into dotnet:mainApr 4, 2023
@EgorBo
EgorBo deleted the stackalloc-zero-simd branch April 4, 2023 23:36
@ghostghost locked as resolved and limited conversation to collaborators May 5, 2023
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.

JIT should vectorize zeroing constant stackalloc-s

6 participants

@EgorBo@stephentoub@benaadams@tannergooding@BruceForstall@SingleAccretion