Uh oh!
There was an error while loading. Please reload this page.
Don't use an array when a simple shift/bit scan will do - #79493
Don't use an array when a simple shift/bit scan will do#79493tannergooding wants to merge 1 commit into
Conversation
ghost
commented
Dec 10, 2022
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch Issue Details
On MSVC, the codegen changes from: ?idOpSize@instrDesc@emitter@@QEBA?AW4emitAttr@@XZ: ; instrDesc::idOpSize() 00000001800F0AA0: 8B 01moveax,dword ptr [rcx] 00000001800F0AA2: 48 8D 0D F7 8C 05learcx,[?emitSizeDecode@emitter@@1QBW4emitAttr@@B]00 00000001800F0AA9: 48 C1 E8 15shrrax,15h 00000001800F0AAD: 83 E0 07andeax,7 00000001800F0AB0: 8B 0481moveax,dword ptr [rcx+rax*4] 00000001800F0AB3: C3 ret?idOpSize@instrDesc@emitter@@QEAAXW4emitAttr@@@Z: ; instrDesc::idOpSize(emitAttr opsz) 0000000180114EC0: 8121 FF FF 1F FF and dword ptr [rcx],0FF1FFFFFh 0000000180114EC6: 4863 C2 movsxdrax,edx 0000000180114EC9: 48 8D 15504803leardx,[?emitSizeEncode@emitter@@1QBW4opSize@1@B]00 0000000180114ED0: 8B 5482 FC movedx,dword ptr [rdx+rax*4-4] 0000000180114ED4: 83 E2 07andedx,7 0000000180114ED7: C1 E2 15shledx,15h 0000000180114EDA: 0911or dword ptr [rcx],edx 0000000180114EDC: C3 retTo: ?idOpSize@instrDesc@emitter@@QEBA?AW4emitAttr@@XZ: ; instrDesc::idOpSize() 00000001800F0A70: 8B 09movecx,dword ptr [rcx] 00000001800F0A72: B8 01000000moveax,1 00000001800F0A77: C1 E9 15shrecx,15h 00000001800F0A7A: 83 E1 07andecx,7 00000001800F0A7D: D3 E0 shleax,cl 00000001800F0A7F: C3 ret?idOpSize@instrDesc@emitter@@QEAAXW4emitAttr@@@Z: ; instrDesc::idOpSize(emitAttr opsz) 0000000180114E70: 8121 FF FF 1F FF and dword ptr [rcx],0FF1FFFFFh 0000000180114E76: 0F BC C2 bsfeax,edx 0000000180114E79: 83 E0 07andeax,7 0000000180114E7C: C1 E0 15shleax,15h 0000000180114E7F: 0901or dword ptr [rcx],eax 0000000180114E81: C3 retAs you can see, for The memory accesses, even in the best case scenario where they were cached, ended up being fairly complex both in terms of the addressing modes required to resolve them but also in the number of indirections required to access the data. Each indirection, even in the case of L1 data would take approx 4 cycles to resolve. This also marks
|
tannergooding
commented
Dec 10, 2022
Hmmm, this helped Linux x86 but hurt other platforms. Was unexpected based on local perf checks. |
instrDesc::idOpSize()andinstrDesc::idOpSize(emitAttr opsz)were using an array lookup when a simple shift or bit scan would have sufficed since all inputs are powers of two.On MSVC, the codegen changes from:
To:
As you can see, for
idOpSizerather than doing 3 memory lookups we now just do the one lookup and ashl. Likewise, foridOpSize(emitAttr)rather than doing 4 memory accesses we now just do 2 with a simplebsf(effectively alzcnt).The memory accesses, even in the best case scenario where they were cached, ended up being fairly complex both in terms of the addressing modes required to resolve them but also in the number of indirections required to access the data. Each indirection, even in the case of L1 data would take approx 4 cycles to resolve.
shlandbsfin comparison are both highly optimized instructions that typically take 1-4 cycles to compute (depending on target CPU).This also marks
idOpSize()asconstso the compiler can understand it is non-mutating and only reading a field.