Proof of the hardware/software company conspiracy

[brew]

Code Select Expand


unsigned int GetIcon(unsigned long client, unsigned long flags) {
unsigned int rettemp = 0;
switch (client) {
case CLIENT_CHAT: //0x43484154:
rettemp++;
case CLIENT_W3XP: //0x57335850:
rettemp++;
case CLIENT_WAR3: //0x57415233:
rettemp++;
case CLIENT_W2BN: //0x5732424E:
rettemp++;
case CLIENT_D2XP: //0x44325850:
rettemp++;
case CLIENT_D2DV: //0x44324456:
rettemp++;
case CLIENT_DSHR: //0x44534852:
rettemp++;
case CLIENT_DRTL: //0x4452544C:
rettemp++;
case CLIENT_JSTR: //0x4A535452:
rettemp++;
case CLIENT_SSHR: //0x53534852:
rettemp++;
case CLIENT_SEXP: //0x53455850:
rettemp++;
}
if ((flags & 1) == 1)
rettemp = 12;
if ((flags & 2) == 2)
rettemp = 13;
if ((flags & 4) == 4)
rettemp = 14;
if ((flags & 8) == 8)
rettemp = 15;
if ((flags & 32) == 32)
rettemp = 16;
if ((flags & 64) == 64)
rettemp = 17;
return rettemp;
}


using MSVC6, compiles to:

Code Select Expand


0040B540   55               PUSH EBP
0040B541   8BEC             MOV EBP,ESP
0040B543   51               PUSH ECX
0040B544   51               PUSH ECX
0040B545   8365 FC 00       AND DWORD PTR SS:[EBP-4],0
0040B549   8B45 08          MOV EAX,DWORD PTR SS:[EBP+8]
0040B54C   8945 F8          MOV DWORD PTR SS:[EBP-8],EAX
0040B54F   817D F8 5254534A CMP DWORD PTR SS:[EBP-8],4A535452
0040B556   77 3C            JA SHORT 0040B594
0040B558   817D F8 5254534A CMP DWORD PTR SS:[EBP-8],4A535452
0040B55F   0F84 96000000    JE 0040B5FB
0040B565   817D F8 54414843 CMP DWORD PTR SS:[EBP-8],43484154
0040B56C   74 55            JE SHORT 0040B5C3
0040B56E   817D F8 56443244 CMP DWORD PTR SS:[EBP-8],44324456
0040B575   74 6F            JE SHORT 0040B5E6
0040B577   817D F8 50583244 CMP DWORD PTR SS:[EBP-8],44325850
0040B57E   74 5F            JE SHORT 0040B5DF
0040B580   817D F8 4C545244 CMP DWORD PTR SS:[EBP-8],4452544C
0040B587   74 6B            JE SHORT 0040B5F4
0040B589   817D F8 52485344 CMP DWORD PTR SS:[EBP-8],44534852
0040B590   74 5B            JE SHORT 0040B5ED
0040B592   EB 7C            JMP SHORT 0040B610
0040B594   817D F8 50584553 CMP DWORD PTR SS:[EBP-8],53455850
0040B59B   74 6C            JE SHORT 0040B609
0040B59D   817D F8 52485353 CMP DWORD PTR SS:[EBP-8],53534852
0040B5A4   74 5C            JE SHORT 0040B602
0040B5A6   817D F8 4E423257 CMP DWORD PTR SS:[EBP-8],5732424E
0040B5AD   74 29            JE SHORT 0040B5D8
0040B5AF   817D F8 50583357 CMP DWORD PTR SS:[EBP-8],57335850
0040B5B6   74 12            JE SHORT 0040B5CA
0040B5B8   817D F8 33524157 CMP DWORD PTR SS:[EBP-8],57415233
0040B5BF   74 10            JE SHORT 0040B5D1
0040B5C1   EB 4D            JMP SHORT 0040B610
0040B5C3   8B45 FC          MOV EAX,DWORD PTR SS:[EBP-4]
0040B5C6   40               INC EAX
0040B5C7   8945 FC          MOV DWORD PTR SS:[EBP-4],EAX
0040B5CA   8B45 FC          MOV EAX,DWORD PTR SS:[EBP-4]
0040B5CD   40               INC EAX
0040B5CE   8945 FC          MOV DWORD PTR SS:[EBP-4],EAX
0040B5D1   8B45 FC          MOV EAX,DWORD PTR SS:[EBP-4]
0040B5D4   40               INC EAX
0040B5D5   8945 FC          MOV DWORD PTR SS:[EBP-4],EAX
0040B5D8   8B45 FC          MOV EAX,DWORD PTR SS:[EBP-4]
0040B5DB   40               INC EAX
0040B5DC   8945 FC          MOV DWORD PTR SS:[EBP-4],EAX
0040B5DF   8B45 FC          MOV EAX,DWORD PTR SS:[EBP-4]
0040B5E2   40               INC EAX
0040B5E3   8945 FC          MOV DWORD PTR SS:[EBP-4],EAX
0040B5E6   8B45 FC          MOV EAX,DWORD PTR SS:[EBP-4]
0040B5E9   40               INC EAX
0040B5EA   8945 FC          MOV DWORD PTR SS:[EBP-4],EAX
0040B5ED   8B45 FC          MOV EAX,DWORD PTR SS:[EBP-4]
0040B5F0   40               INC EAX
0040B5F1   8945 FC          MOV DWORD PTR SS:[EBP-4],EAX
0040B5F4   8B45 FC          MOV EAX,DWORD PTR SS:[EBP-4]
0040B5F7   40               INC EAX
0040B5F8   8945 FC          MOV DWORD PTR SS:[EBP-4],EAX
0040B5FB   8B45 FC          MOV EAX,DWORD PTR SS:[EBP-4]
0040B5FE   40               INC EAX
0040B5FF   8945 FC          MOV DWORD PTR SS:[EBP-4],EAX
0040B602   8B45 FC          MOV EAX,DWORD PTR SS:[EBP-4]
0040B605   40               INC EAX
0040B606   8945 FC          MOV DWORD PTR SS:[EBP-4],EAX
0040B609   8B45 FC          MOV EAX,DWORD PTR SS:[EBP-4]
0040B60C   40               INC EAX
0040B60D   8945 FC          MOV DWORD PTR SS:[EBP-4],EAX
0040B610   8B45 0C          MOV EAX,DWORD PTR SS:[EBP+C]
0040B613   83E0 01          AND EAX,1
0040B616   83F8 01          CMP EAX,1
0040B619   75 07            JNZ SHORT 0040B622
0040B61B   C745 FC 0C000000 MOV DWORD PTR SS:[EBP-4],0C
0040B622   8B45 0C          MOV EAX,DWORD PTR SS:[EBP+C]
0040B625   83E0 02          AND EAX,2
0040B628   83F8 02          CMP EAX,2
0040B62B   75 07            JNZ SHORT 0040B634
0040B62D   C745 FC 0D000000 MOV DWORD PTR SS:[EBP-4],0D
0040B634   8B45 0C          MOV EAX,DWORD PTR SS:[EBP+C]
0040B637   83E0 04          AND EAX,4
0040B63A   83F8 04          CMP EAX,4
0040B63D   75 07            JNZ SHORT 0040B646
0040B63F   C745 FC 0E000000 MOV DWORD PTR SS:[EBP-4],0E
0040B646   8B45 0C          MOV EAX,DWORD PTR SS:[EBP+C]
0040B649   83E0 08          AND EAX,8
0040B64C   83F8 08          CMP EAX,8
0040B64F   75 07            JNZ SHORT 0040B658
0040B651   C745 FC 0F000000 MOV DWORD PTR SS:[EBP-4],0F
0040B658   8B45 0C          MOV EAX,DWORD PTR SS:[EBP+C]
0040B65B   83E0 20          AND EAX,20
0040B65E   83F8 20          CMP EAX,20
0040B661   75 07            JNZ SHORT 0040B66A
0040B663   C745 FC 10000000 MOV DWORD PTR SS:[EBP-4],10
0040B66A   8B45 0C          MOV EAX,DWORD PTR SS:[EBP+C]
0040B66D   83E0 40          AND EAX,40
0040B670   83F8 40          CMP EAX,40
0040B673   75 07            JNZ SHORT 0040B67C
0040B675   C745 FC 11000000 MOV DWORD PTR SS:[EBP-4],11
0040B67C   8B45 FC          MOV EAX,DWORD PTR SS:[EBP-4]
0040B67F   C9               LEAVE
0040B680   C3               RETN

In release mode too. With all optimizations turned on.

* brew throws up

[UserLoser]

cool

[MyndFyre]

Your code sucks.  You should have the flags check before the switch block, since the flags make the switch totally irrelevant.

Also, the fact that you're using fallthrough is probably what's preventing it from breaking it into one of the other optimizations.  For this number of cases, I wouldn't have expected to see it use a jump table, but rather a divide-and-conquer strategy.  But, since you're using fallthrough, divide-and-conquer wouldn't work (because, for instance, CLIENT_CHAT wouldn't get the right number of values.

I bet the optimization (jump table or divide-and-conquer) would kick in if you just used constants with break statements.

I was going to suggest that you might benefit from using a templated STL map (should be close to an O(1) lookup), but since I recalled that you don't like code reuse or software engineering beyond the 1940's, that may not be your desired route.

[Warrior]

Proof of brew's retardation

[warz]

Just another bit of proof, among hundreds.

[brew]

You should have the flags check before the switch block, since the flags make the switch totally irrelevant.

that's true. I never really thought of that (or even looked at it until you mentioned that). Thanks.

Also, the fact that you're using fallthrough is probably what's preventing it from breaking it into one of the other optimizations.  For this number of cases, I wouldn't have expected to see it use a jump table, but rather a divide-and-conquer strategy.  But, since you're using fallthrough, divide-and-conquer wouldn't work (because, for instance, CLIENT_CHAT wouldn't get the right number of values.

I bet the optimization (jump table or divide-and-conquer) would kick in if you just used constants with break statements.

A switch statement with all those constants didn't look right to me at the time i guess.
BUT THE POINT IS that it shouldn'tve placed two movs after every inc, I would've kept that value in eax, but do all the and calculations with ecx. saves a lot of shit. Now that I look at it after what you've said, i think i know what it's trying to do, but just fails hard at doing it. The compiler also does so much more retarded shit, for example executes LEA twice on the same thing like so:

Code Select Expand


0040ABC4   8D85 04FAFFFF    LEA EAX,DWORD PTR SS:[EBP-5FC]
0040ABCA   50               PUSH EAX
0040ABCB   8D85 04FAFFFF    LEA EAX,DWORD PTR SS:[EBP-5FC]
0040ABD1   50               PUSH EAX
0040ABD2   E8 D60B0000      CALL 0040B7AD

my precious 6 bytes of space. wasted.

I was going to suggest that you might benefit from using a templated STL map (should be close to an O(1) lookup), but since I recalled that you don't like code reuse or software engineering beyond the 1940's, that may not be your desired route.

You're right. C++ isn't my thing.

[brew]

grr. okay, so i did what myndfyre suggested. now it looks like this:

Code Select Expand


unsigned int GetIcon(unsigned long client, unsigned long flags) {
unsigned int rettemp;
if (flags & 64)
return 17;
if (flags & 32)
return 16;
if (flags & 8)
return 15;
if (flags & 4)
return 14;
if (flags & 2)
return 13;
if (flags & 1)
return 12;
switch (client) {
case CLIENT_CHAT: //0x43484154:
return 11;
case CLIENT_W3XP: //0x57335850:
return 10;
case CLIENT_WAR3: //0x57415233:
return 9;
case CLIENT_W2BN: //0x5732424E:
return 8;
case CLIENT_D2XP: //0x44325850:
return 7;
case CLIENT_D2DV: //0x44324456:
return 6;
case CLIENT_DSHR: //0x44534852:
return 5;
case CLIENT_DRTL: //0x4452544C:
return 4;
case CLIENT_JSTR: //0x4A535452:
return 3;
case CLIENT_SSHR: //0x53534852:
return 2;
case CLIENT_SEXP: //0x53455850:
return 1;
}
}


Code Select Expand


0040B390   55               PUSH EBP
0040B391   8BEC             MOV EBP,ESP
0040B393   51               PUSH ECX
0040B394   51               PUSH ECX
0040B395   8B45 0C          MOV EAX,DWORD PTR SS:[EBP+C]
0040B398   83E0 40          AND EAX,40
0040B39B   83F8 40          CMP EAX,40
0040B39E   75 08            JNZ SHORT 0040B3A8
0040B3A0   6A 11            PUSH 11
0040B3A2   58               POP EAX
0040B3A3   E9 17010000      JMP 0040B4BF


0040B3A8   8B45 0C          MOV EAX,DWORD PTR SS:[EBP+C]
0040B3AB   83E0 20          AND EAX,20
0040B3AE   83F8 20          CMP EAX,20
0040B3B1   75 08            JNZ SHORT 0040B3BB
0040B3B3   6A 10            PUSH 10
0040B3B5   58               POP EAX
0040B3B6   E9 04010000      JMP 0040B4BF

0040B3BB   8B45 0C          MOV EAX,DWORD PTR SS:[EBP+C]
0040B3BE   83E0 08          AND EAX,8
0040B3C1   83F8 08          CMP EAX,8
0040B3C4   75 08            JNZ SHORT 0040B3CE
0040B3C6   6A 0F            PUSH 0F
0040B3C8   58               POP EAX
0040B3C9   E9 F1000000      JMP 0040B4BF

0040B3CE   8B45 0C          MOV EAX,DWORD PTR SS:[EBP+C]
0040B3D1   83E0 04          AND EAX,4
0040B3D4   83F8 04          CMP EAX,4
0040B3D7   75 08            JNZ SHORT 0040B3E1
0040B3D9   6A 0E            PUSH 0E
0040B3DB   58               POP EAX
0040B3DC   E9 DE000000      JMP 0040B4BF

0040B3E1   8B45 0C          MOV EAX,DWORD PTR SS:[EBP+C]
0040B3E4   83E0 02          AND EAX,2
0040B3E7   83F8 02          CMP EAX,2
0040B3EA   75 08            JNZ SHORT 0040B3F4
0040B3EC   6A 0D            PUSH 0D
0040B3EE   58               POP EAX
0040B3EF   E9 CB000000      JMP 0040B4BF

0040B3F4   8B45 0C          MOV EAX,DWORD PTR SS:[EBP+C]
0040B3F7   83E0 01          AND EAX,1
0040B3FA   83F8 01          CMP EAX,1
0040B3FD   75 08            JNZ SHORT 0040B407
0040B3FF   6A 0C            PUSH 0C
0040B401   58               POP EAX
0040B402   E9 B8000000      JMP 0040B4BF

0040B407   8365 FC 00       AND DWORD PTR SS:[EBP-4],0
0040B40B   8B45 08          MOV EAX,DWORD PTR SS:[EBP+8]
0040B40E   8945 F8          MOV DWORD PTR SS:[EBP-8],EAX
0040B411   817D F8 5254534A CMP DWORD PTR SS:[EBP-8],4A535452
0040B418   77 3C            JA SHORT 0040B456
0040B41A   817D F8 5254534A CMP DWORD PTR SS:[EBP-8],4A535452
0040B421   0F84 86000000    JE 0040B4AD
0040B427   817D F8 54414843 CMP DWORD PTR SS:[EBP-8],43484154
0040B42E   74 55            JE SHORT 0040B485
0040B430   817D F8 56443244 CMP DWORD PTR SS:[EBP-8],44324456
0040B437   74 65            JE SHORT 0040B49E
0040B439   817D F8 50583244 CMP DWORD PTR SS:[EBP-8],44325850
0040B440   74 57            JE SHORT 0040B499
0040B442   817D F8 4C545244 CMP DWORD PTR SS:[EBP-8],4452544C
0040B449   74 5D            JE SHORT 0040B4A8
0040B44B   817D F8 52485344 CMP DWORD PTR SS:[EBP-8],44534852
0040B452   74 4F            JE SHORT 0040B4A3
0040B454   EB 66            JMP SHORT 0040B4BC
0040B456   817D F8 50584553 CMP DWORD PTR SS:[EBP-8],53455850
0040B45D   74 58            JE SHORT 0040B4B7
0040B45F   817D F8 52485353 CMP DWORD PTR SS:[EBP-8],53534852
0040B466   74 4A            JE SHORT 0040B4B2
0040B468   817D F8 4E423257 CMP DWORD PTR SS:[EBP-8],5732424E
0040B46F   74 23            JE SHORT 0040B494
0040B471   817D F8 50583357 CMP DWORD PTR SS:[EBP-8],57335850
0040B478   74 10            JE SHORT 0040B48A
0040B47A   817D F8 33524157 CMP DWORD PTR SS:[EBP-8],57415233
0040B481   74 0C            JE SHORT 0040B48F
0040B483   EB 37            JMP SHORT 0040B4BC
0040B485   6A 0B            PUSH 0B
0040B487   58               POP EAX
0040B488   EB 35            JMP SHORT 0040B4BF
0040B48A   6A 0A            PUSH 0A
0040B48C   58               POP EAX
0040B48D   EB 30            JMP SHORT 0040B4BF
0040B48F   6A 09            PUSH 9
0040B491   58               POP EAX
0040B492   EB 2B            JMP SHORT 0040B4BF
0040B494   6A 08            PUSH 8
0040B496   58               POP EAX
0040B497   EB 26            JMP SHORT 0040B4BF
0040B499   6A 07            PUSH 7
0040B49B   58               POP EAX
0040B49C   EB 21            JMP SHORT 0040B4BF
0040B49E   6A 06            PUSH 6
0040B4A0   58               POP EAX
0040B4A1   EB 1C            JMP SHORT 0040B4BF
0040B4A3   6A 05            PUSH 5
0040B4A5   58               POP EAX
0040B4A6   EB 17            JMP SHORT 0040B4BF
0040B4A8   6A 04            PUSH 4
0040B4AA   58               POP EAX
0040B4AB   EB 12            JMP SHORT 0040B4BF
0040B4AD   6A 03            PUSH 3
0040B4AF   58               POP EAX
0040B4B0   EB 0D            JMP SHORT 0040B4BF
0040B4B2   6A 02            PUSH 2
0040B4B4   58               POP EAX
0040B4B5   EB 08            JMP SHORT 0040B4BF
0040B4B7   6A 01            PUSH 1
0040B4B9   58               POP EAX
0040B4BA   EB 03            JMP SHORT 0040B4BF
0040B4BC   8B45 FC          MOV EAX,DWORD PTR SS:[EBP-4]
0040B4BF   C9               LEAVE
0040B4C0   C3               RETN

is it just me, or was my original break-through switch cases a better idea? that is, if the compiler actually did it right. I can't get it to make a hash table instead of sequential cmp and jes.
And if you've noticed, it's doing
push imm8, sign extends to 32 bit
pop register
in order to move w/e constant value into eax.
this is pretty small, (3 bytes) especially considering that mov can't take 8 bit values. movsx can, but it still turns out to be pretty big. This is obviously a size optimization, but how about speed? I've heard somewhere that xor eax, eax lea eax, [eax + blah] is the FASTEST way. Can anyone verify this?

[Kp]

Have you tried with a compiler from the last five years? :p  I just put your code through gcc 4.1.2 and it came out with something more in line with what you seem to want.

[brew]

Have you tried with a compiler from the last five years? :p  I just put your code through gcc 4.1.2 and it came out with something more in line with what you seem to want.

I haven't. However, i've been thinking of switching to Watcom C. I heard it generates the fastest code of all the modern compilers. For now.... I think it would be optimal if I use inline assembly for this, using fastcall as the calling convention.

EDIT**
How many ms can you get it down to for calling this function 10 million times, passing CLIENT_D2DV and 0? For me, if i use __fastcall, exactly 297 in debug, and 79 in release.

[MyndFyre]

Well, the only other reason I can think of for the double MOV is thread-safety, where another thread may have updated the memory location but not the register (because a thread context switch wouldn't impact the register value).  But, the code isn't exactly reentrant anyway.

Compiling with VC 2008, with small-program optimization favored, this is what is generated (note, this is compiler-generated machine+assembly, not from IDA or a similar tool):

Code Select Expand


_GetIcon PROC ; COMDAT
  00000 8a 44 24 08 mov al, BYTE PTR _flags$[esp-4]    ;if (flags & 64)
  00004 a8 40 test al, 64 ; 00000040H
  00006 74 04 je SHORT $LN19@GetIcon
  00008 6a 11 push 17 ; 00000011H
  0000a 58 pop eax                                         ; return 17;
  0000b c3 ret 0
$LN19@GetIcon:
  0000c a8 20 test al, 32 ; 00000020H
  0000e 74 04 je SHORT $LN18@GetIcon
  00010 6a 10 push 16 ; 00000010H
  00012 58 pop eax
  00013 c3 ret 0
$LN18@GetIcon:
  00014 a8 08 test al, 8
  00016 74 04 je SHORT $LN17@GetIcon
  00018 6a 0f push 15 ; 0000000fH
  0001a 58 pop eax
  0001b c3 ret 0
$LN17@GetIcon:
  0001c a8 04 test al, 4
  0001e 74 04 je SHORT $LN16@GetIcon
  00020 6a 0e push 14 ; 0000000eH
  00022 58 pop eax
  00023 c3 ret 0
$LN16@GetIcon:
  00024 a8 02 test al, 2
  00026 74 04 je SHORT $LN15@GetIcon
  00028 6a 0d push 13 ; 0000000dH
  0002a 58 pop eax
  0002b c3 ret 0
$LN15@GetIcon:                                ; if (flags & 1)
  0002c a8 01 test al, 1
  0002e 74 04 je SHORT $LN14@GetIcon
  00030 6a 0c push 12 ; 0000000cH
  00032 58 pop eax
  00033 c3 ret 0
$LN14@GetIcon:                      ; now we're out of the flags checking - this is clearly a divide-and-conquer strategy
  00034 8b 4c 24 04 mov ecx, DWORD PTR _client$[esp-4]
  00038 b8 52 54 53 4a mov eax, 1246975058 ; 4a535452H
  0003d 3b c8 cmp ecx, eax
  0003f 77 42 ja SHORT $LN22@GetIcon  ; if client is w3xp, war3, w2bn, sshr, sexp
  00041 74 3c je SHORT $LN3@GetIcon    ; if client is jstr
  00043 81 f9 54 41 48
43 cmp ecx, 1128808788 ; 43484154H
  00049 74 30 je SHORT $LN11@GetIcon  ; if client is chat
  0004b 81 f9 56 44 32
44 cmp ecx, 1144144982 ; 44324456H
  00051 74 24 je SHORT $LN6@GetIcon    ; if client is d2dv
  00053 81 f9 50 58 32
44 cmp ecx, 1144150096 ; 44325850H
  00059 74 18 je SHORT $LN7@GetIcon    ; if client is d2xp
  0005b 81 f9 4c 54 52
44 cmp ecx, 1146246220 ; 4452544cH
  00061 74 0c je SHORT $LN4@GetIcon    ; if client is drtl
  00063 81 f9 52 48 53
44 cmp ecx, 1146308690 ; 44534852H
  00069 75 53 jne SHORT $LN12@GetIcon  ; if no match
  0006b 6a 05 push 5    ; if client is dshr
  0006d 58 pop eax
  0006e c3 ret 0
$LN4@GetIcon:
  0006f 6a 04 push 4
  00071 58 pop eax
  00072 c3 ret 0
$LN7@GetIcon:
  00073 6a 07 push 7
  00075 58 pop eax
  00076 c3 ret 0
$LN6@GetIcon:
  00077 6a 06 push 6
  00079 58 pop eax
  0007a c3 ret 0
$LN11@GetIcon:
  0007b 6a 0b push 11 ; 0000000bH
  0007d 58 pop eax
  0007e c3 ret 0
$LN3@GetIcon:
  0007f 6a 03 push 3
  00081 58 pop eax
  00082 c3 ret 0
$LN22@GetIcon:
  00083 81 f9 50 58 45
53 cmp ecx, 1397053520 ; 53455850H
  00089 74 30 je SHORT $LN1@GetIcon    ; sexp
  0008b 81 f9 52 48 53
53 cmp ecx, 1397966930 ; 53534852H
  00091 74 24 je SHORT $LN2@GetIcon    ; sshr
  00093 81 f9 4e 42 32
57 cmp ecx, 1462911566 ; 5732424eH
  00099 74 18 je SHORT $LN8@GetIcon    ; w2bn
  0009b 81 f9 50 58 33
57 cmp ecx, 1462982736 ; 57335850H
  000a1 74 0c je SHORT $LN10@GetIcon  ; w3xp
  000a3 81 f9 33 52 41
57 cmp ecx, 1463898675 ; 57415233H
  000a9 75 13 jne SHORT $LN12@GetIcon  ; no match
  000ab 6a 09 push 9                                     ; war3
  000ad 58 pop eax
  000ae c3 ret 0
$LN10@GetIcon:
  000af 6a 0a push 10 ; 0000000aH
  000b1 58 pop eax
  000b2 c3 ret 0
$LN8@GetIcon:
  000b3 6a 08 push 8
  000b5 58 pop eax
  000b6 c3 ret 0
$LN2@GetIcon:
  000b7 6a 02 push 2
  000b9 58 pop eax
  000ba c3 ret 0
$LN1@GetIcon:
  000bb 33 c0 xor eax, eax
  000bd 40 inc eax
$LN12@GetIcon:
  000be c3 ret 0
_GetIcon ENDP

That seems pretty tight to me considering all of the short jumps; no thrashing the processor instruction queue (in fact I bet the processor can probably execute a lot of these simultaneously).

The theme of optimization for fast code (as opposed to small) seems to be moving whole machine words into registers, rather than pushing and popping.  For example, mov eax, 17 (the code if flags & 64), is b8 11 00 00 00, whereas push 17 / pop eax is 6a 11 58, so a savings of two bytes is achieved by optimizing for code size.

Someone who knows: is it possible to do a short jump by index?

Besides, since the key to your hash table would be one of a few very large numbers, you'd probably have difficulty unless you made a clever hashing function, and that'd be pretty wasteful.  Jump tables seem to be most useful when the numbers are sequential, or at least relatively close together.

[TheMinistered]

I think I might also point out how bad brew's code is. I will repost his code below for easy reference as I make my point.

Code Select Expand


unsigned int GetIcon(unsigned long client, unsigned long flags) {
unsigned int rettemp = 0;
switch (client) {
case CLIENT_CHAT: //0x43484154:
rettemp++;
case CLIENT_W3XP: //0x57335850:
rettemp++;
case CLIENT_WAR3: //0x57415233:
rettemp++;
case CLIENT_W2BN: //0x5732424E:
rettemp++;
case CLIENT_D2XP: //0x44325850:
rettemp++;
case CLIENT_D2DV: //0x44324456:
rettemp++;
case CLIENT_DSHR: //0x44534852:
rettemp++;
case CLIENT_DRTL: //0x4452544C:
rettemp++;
case CLIENT_JSTR: //0x4A535452:
rettemp++;
case CLIENT_SSHR: //0x53534852:
rettemp++;
case CLIENT_SEXP: //0x53455850:
rettemp++;
}
if ((flags & 1) == 1)
rettemp = 12;
if ((flags & 2) == 2)
rettemp = 13;
if ((flags & 4) == 4)
rettemp = 14;
if ((flags & 8) == 8)
rettemp = 15;
if ((flags & 32) == 32)
rettemp = 16;
if ((flags & 64) == 64)
rettemp = 17;
return rettemp;
}


Well really I have only one good point. I don't think anyone touched on this directly, but they beat around the bush a little.  In my opinion, that switch statement shouldn't be there at all. The statement itself has no part in "getting the icon" so it is just extra code that does nothing really, as you can see rettemp is later written over in one of the following if statements.

So assuming his code actually works, this should be more effecient.

Code Select Expand


unsigned int GetIcon(unsigned long flags) {
       unsigned int rettemp = 1;

if ((flags & 1) == 1)
rettemp  = 12;
if ((flags & 2) == 2)
rettemp = 13;
if ((flags & 4) == 4)
rettemp = 14;
if ((flags & 8) == 8)
rettemp = 15;
if ((flags & 32) == 32)
rettemp = 16;
if ((flags & 64) == 64)
rettemp = 17;
return rettemp;
}


Hell, you really shouldn't even need a local variable to store the result in.  You could probaly just do return #;

[l2k-Shadow]

actually... the switch statement does change it. Since he doesn't break after his case. If the client is CLIENT_CHAT, it would rettemp++ as many times as there are cases, ending up with rettemp = 11 at the end. so even if the flags were 0, it would still change it. of course if the flags were not 0, it's useless code.

[FrOzeN]

Being that brew is all about being a perforance nazi, wouldn't you want to be parsing the client/flags by reference? Also, by returning a value once the value is attained breaks it out of the function, which I assume would also be faster than continuing on. Another note, you should probably create more constants as to remove the need of magic numbers. The following should run a little faster:

Code Select Expand

unsigned int GetIcon(unsigned long &client, unsigned long &flags) {
    if (flags & 1) // Blizzard Representative
        return 12;
    if (flags & 8) // Battle.net Administrator
        return 15;
    if (flags & 2) // Channel Operator
        return 13;
    if (flags & 4) // Speaker
        return 14;
    if (flags & 64) // Special Guest
        return 17;
    if (flags & 32) // Squelched
        return 16

    switch (client)
    {
        case CLIENT_CHAT: // 0x43484154
            return 11;
        case CLIENT_W3XP: // 0x57335850
            return 10;
        case CLIENT_WAR3: // 0x57415233
            return 9;
        case CLIENT_W2BN: // 0x5732424E
            return 8;
        case CLIENT_D2XP: // 0x44325850
            return 7;
        case CLIENT_D2DV: // 0x44324456
            return 6;
        case CLIENT_DSHR: // 0x44534852
            return 5;
        case CLIENT_DRTL: // 0x4452544C
            return 4;
        case CLIENT_JSTR: // 0x4A535452
            return 3;
        case CLIENT_SSHR: // 0x53534852
            return 2
        case CLIENT_SEXP: // 0x53455850
            return 1;
    }
    return 0; // Unknown Icon
}

[EDIT] Another thing you could do brew, is to run some statistics on Battle.net, and then determine which game clients are more commonly used. That way you could order your switch statement to have the ones that occur more higher up. Maybe you'll save one or two billionths of millisecond!

[Joe[x86]]

Another thing you could do brew, is to run some statistics on Battle.net, and then determine which game clients are more commonly used. That way you could order your switch statement to have the ones that occur more higher up. Maybe you'll save one or two billionths of millisecond!

hifive.

[MyndFyre]

That would probably be inaccurate, since most bot users will idle in channels that aren't composed of the population represented in public Bnet channels.

Well really I have only one good point. I don't think anyone touched on this directly, but they beat around the bush a little.  In my opinion, that switch statement shouldn't be there at all. The statement itself has no part in "getting the icon" so it is just extra code that does nothing really, as you can see rettemp is later written over in one of the following if statements.

Shadow is correct; the switch statement does work.  However, I'm also in agreement that he should just set/return the values in the switch statement, not rely on fallthrough (which is generally considered poor language practice).