Re: [RFA/i386] Prec x86 MMX 3DNow! SSE SSE2 SSE3 SSSE3 SSE4 support

[Hui Zhu <teawater at gmail dot com>]

On Mon, Mar 29, 2010 at 09:36, Michael Snyder <msnyder@vmware.com> wrote:
> Hui Zhu wrote:
>>
>> Hello,
>>
>> I make a testsute for sse. ?I make it just support x86-64 target
>> because I got insn unsupport trap in a x86-32 pc. ?Maybe the CPU of
>> this pc is too old. ?:)
>>
>> And I found some bug in sse patch. ?I make a new one.
>>
>> Please help me review them. Thanks.
>
> Two minor comments on the code:
>
>
> + ? ?case 0x0faa: ? ?/* rsm */
> + ? ? ?I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_EFLAGS_REGNUM);
> + ? ? ?I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_REAX_REGNUM);
> + ? ? ?I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_RECX_REGNUM);
> + ? ? ?I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_REDX_REGNUM);
> + ? ? ?I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_REBX_REGNUM);
> + ? ? ?I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_RESP_REGNUM);
> + ? ? ?I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_REBP_REGNUM);
> + ? ? ?I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_RESI_REGNUM);
> + ? ? ?I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_REDI_REGNUM);
> + ? ? ?break;
> +
> + ? ?case 0x0fae:
>
> Is there a comment for this case statement?
> Maybe it doesn't need one, but most of the others have one...

fxsave fxrstor ldmxcsr stmxcsr lfence mfence sfence clflush

They looks don't have family name.  And put this line to there will
make this line bigger than 80.  So...

>
> + ? ?/* Add prefix to opcode. ?*/
> + ? ?case 0x0f10:
> + ? ?case 0x0f11:
> + ? ?case 0x0f12:
> + ? ?case 0x0f13:
> + ? ?case 0x0f14:
> + ? ?case 0x0f15:
> + ? ?case 0x0f16:
> + ? ?case 0x0f17:
> + ? ?case 0x0f28:
> + ? ?case 0x0f29:
> + ? ?case 0x0f2a:
> + ? ?case 0x0f2b:
> + ? ?case 0x0f2c:
> + ? ?case 0x0f2d:
> + ? ?case 0x0f2e:
> + ? ?case 0x0f2f:
> + ? ?case 0x0f38:
> + ? ?case 0x0f39:
> + ? ?case 0x0f3a:
> + ? ?case 0x0f50:
> + ? ?case 0x0f51:
> + ? ?case 0x0f52:
> + ? ?case 0x0f53:
> + ? ?case 0x0f54:
> + ? ?case 0x0f55:
> + ? ?case 0x0f56:
> + ? ?case 0x0f57:
> + ? ?case 0x0f58:
> + ? ?case 0x0f59:
> + ? ?case 0x0f5a:
> + ? ?case 0x0f5b:
> + ? ?case 0x0f5c:
> + ? ?case 0x0f5d:
> + ? ?case 0x0f5e:
> + ? ?case 0x0f5f:
> + ? ?case 0x0f60:
> + ? ?case 0x0f61:
> + ? ?case 0x0f62:
> + ? ?case 0x0f63:
> + ? ?case 0x0f64:
> + ? ?case 0x0f65:
> + ? ?case 0x0f66:
> + ? ?case 0x0f67:
> + ? ?case 0x0f68:
> + ? ?case 0x0f69:
> + ? ?case 0x0f6a:
> + ? ?case 0x0f6b:
> + ? ?case 0x0f6c:
> + ? ?case 0x0f6d:
> + ? ?case 0x0f6e:
> + ? ?case 0x0f6f:
> + ? ?case 0x0f70:
> + ? ?case 0x0f71:
> + ? ?case 0x0f72:
> + ? ?case 0x0f73:
> + ? ?case 0x0f74:
> + ? ?case 0x0f75:
> + ? ?case 0x0f76:
> + ? ?case 0x0f7c:
> + ? ?case 0x0f7d:
> + ? ?case 0x0f7e:
> + ? ?case 0x0f7f:
> + ? ?case 0x0fb8:
> + ? ?case 0x0fc2:
> + ? ?case 0x0fc4:
> + ? ?case 0x0fc5:
> + ? ?case 0x0fc6:
> + ? ?case 0x0fd0:
> + ? ?case 0x0fd1:
> + ? ?case 0x0fd2:
> + ? ?case 0x0fd3:
> + ? ?case 0x0fd4:
> + ? ?case 0x0fd5:
> + ? ?case 0x0fd6:
> + ? ?case 0x0fd7:
> + ? ?case 0x0fd8:
> + ? ?case 0x0fd9:
> + ? ?case 0x0fda:
> + ? ?case 0x0fdb:
> + ? ?case 0x0fdc:
> + ? ?case 0x0fdd:
> + ? ?case 0x0fde:
> + ? ?case 0x0fdf:
> + ? ?case 0x0fe0:
> + ? ?case 0x0fe1:
> + ? ?case 0x0fe2:
> + ? ?case 0x0fe3:
> + ? ?case 0x0fe4:
> + ? ?case 0x0fe5:
> + ? ?case 0x0fe6:
> + ? ?case 0x0fe7:
> + ? ?case 0x0fe8:
> + ? ?case 0x0fe9:
> + ? ?case 0x0fea:
> + ? ?case 0x0feb:
> + ? ?case 0x0fec:
> + ? ?case 0x0fed:
> + ? ?case 0x0fee:
> + ? ?case 0x0fef:
> + ? ?case 0x0ff0:
> + ? ?case 0x0ff1:
> + ? ?case 0x0ff2:
> + ? ?case 0x0ff3:
> + ? ?case 0x0ff4:
> + ? ?case 0x0ff5:
> + ? ?case 0x0ff6:
> + ? ?case 0x0ff7:
> + ? ?case 0x0ff8:
> + ? ?case 0x0ff9:
> + ? ?case 0x0ffa:
> + ? ?case 0x0ffb:
> + ? ?case 0x0ffc:
> + ? ?case 0x0ffd:
> + ? ?case 0x0ffe:
>
> There's 114 lines that could be replaced by a single "if",
> just by moving them into the default case block.
>
>
>
>

0x0f10 ... 0x0f2f
0x0f38 ... 0x0f3a
0x0f50 ... 0x0f76
0x0f7c ... 0x0f7f
0x0fb8
0x0fc2
0x0fc4 ... 0x0fc6
0x0fd0 ... 0x0ffe

Looks single "if" cannot handle it.
I suggest let compiler to do this optimization job.

Thanks,
Hui