From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 29223 invoked by alias); 29 Mar 2010 09:25:12 -0000 Received: (qmail 29210 invoked by uid 22791); 29 Mar 2010 09:25:11 -0000 X-SWARE-Spam-Status: No, hits=-1.4 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FROM,RCVD_IN_DNSWL_NONE,SARE_MSGID_LONG40 X-Spam-Check-By: sourceware.org Received: from mail-pw0-f41.google.com (HELO mail-pw0-f41.google.com) (209.85.160.41) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 29 Mar 2010 09:25:06 +0000 Received: by pwi2 with SMTP id 2so3662194pwi.0 for ; Mon, 29 Mar 2010 02:25:04 -0700 (PDT) MIME-Version: 1.0 Received: by 10.143.40.6 with HTTP; Mon, 29 Mar 2010 02:24:43 -0700 (PDT) In-Reply-To: <4BB00426.40702@vmware.com> References: <4B26825B.7000209@vmware.com> <4B2BDAEC.7090207@vmware.com> <20100109122833.GB2007@adacore.com> <4B4BD2AE.7030405@vmware.com> <4BB00426.40702@vmware.com> From: Hui Zhu Date: Mon, 29 Mar 2010 09:25:00 -0000 Received: by 10.142.250.10 with SMTP id x10mr1775423wfh.341.1269854704111; Mon, 29 Mar 2010 02:25:04 -0700 (PDT) Message-ID: Subject: Re: [RFA/i386] Prec x86 MMX 3DNow! SSE SSE2 SSE3 SSSE3 SSE4 support To: Michael Snyder Cc: Joel Brobecker , gdb-patches ml Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2010-03/txt/msg00980.txt.bz2 On Mon, Mar 29, 2010 at 09:36, Michael Snyder wrote: > Hui Zhu wrote: >> >> Hello, >> >> I make a testsute for sse. =A0I make it just support x86-64 target >> because I got insn unsupport trap in a x86-32 pc. =A0Maybe the CPU of >> this pc is too old. =A0:) >> >> And I found some bug in sse patch. =A0I make a new one. >> >> Please help me review them. Thanks. > > Two minor comments on the code: > > > + =A0 =A0case 0x0faa: =A0 =A0/* rsm */ > + =A0 =A0 =A0I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_EFLAGS_REGNUM); > + =A0 =A0 =A0I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_REAX_REGNUM); > + =A0 =A0 =A0I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_RECX_REGNUM); > + =A0 =A0 =A0I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_REDX_REGNUM); > + =A0 =A0 =A0I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_REBX_REGNUM); > + =A0 =A0 =A0I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_RESP_REGNUM); > + =A0 =A0 =A0I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_REBP_REGNUM); > + =A0 =A0 =A0I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_RESI_REGNUM); > + =A0 =A0 =A0I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_REDI_REGNUM); > + =A0 =A0 =A0break; > + > + =A0 =A0case 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... > > + =A0 =A0/* Add prefix to opcode. =A0*/ > + =A0 =A0case 0x0f10: > + =A0 =A0case 0x0f11: > + =A0 =A0case 0x0f12: > + =A0 =A0case 0x0f13: > + =A0 =A0case 0x0f14: > + =A0 =A0case 0x0f15: > + =A0 =A0case 0x0f16: > + =A0 =A0case 0x0f17: > + =A0 =A0case 0x0f28: > + =A0 =A0case 0x0f29: > + =A0 =A0case 0x0f2a: > + =A0 =A0case 0x0f2b: > + =A0 =A0case 0x0f2c: > + =A0 =A0case 0x0f2d: > + =A0 =A0case 0x0f2e: > + =A0 =A0case 0x0f2f: > + =A0 =A0case 0x0f38: > + =A0 =A0case 0x0f39: > + =A0 =A0case 0x0f3a: > + =A0 =A0case 0x0f50: > + =A0 =A0case 0x0f51: > + =A0 =A0case 0x0f52: > + =A0 =A0case 0x0f53: > + =A0 =A0case 0x0f54: > + =A0 =A0case 0x0f55: > + =A0 =A0case 0x0f56: > + =A0 =A0case 0x0f57: > + =A0 =A0case 0x0f58: > + =A0 =A0case 0x0f59: > + =A0 =A0case 0x0f5a: > + =A0 =A0case 0x0f5b: > + =A0 =A0case 0x0f5c: > + =A0 =A0case 0x0f5d: > + =A0 =A0case 0x0f5e: > + =A0 =A0case 0x0f5f: > + =A0 =A0case 0x0f60: > + =A0 =A0case 0x0f61: > + =A0 =A0case 0x0f62: > + =A0 =A0case 0x0f63: > + =A0 =A0case 0x0f64: > + =A0 =A0case 0x0f65: > + =A0 =A0case 0x0f66: > + =A0 =A0case 0x0f67: > + =A0 =A0case 0x0f68: > + =A0 =A0case 0x0f69: > + =A0 =A0case 0x0f6a: > + =A0 =A0case 0x0f6b: > + =A0 =A0case 0x0f6c: > + =A0 =A0case 0x0f6d: > + =A0 =A0case 0x0f6e: > + =A0 =A0case 0x0f6f: > + =A0 =A0case 0x0f70: > + =A0 =A0case 0x0f71: > + =A0 =A0case 0x0f72: > + =A0 =A0case 0x0f73: > + =A0 =A0case 0x0f74: > + =A0 =A0case 0x0f75: > + =A0 =A0case 0x0f76: > + =A0 =A0case 0x0f7c: > + =A0 =A0case 0x0f7d: > + =A0 =A0case 0x0f7e: > + =A0 =A0case 0x0f7f: > + =A0 =A0case 0x0fb8: > + =A0 =A0case 0x0fc2: > + =A0 =A0case 0x0fc4: > + =A0 =A0case 0x0fc5: > + =A0 =A0case 0x0fc6: > + =A0 =A0case 0x0fd0: > + =A0 =A0case 0x0fd1: > + =A0 =A0case 0x0fd2: > + =A0 =A0case 0x0fd3: > + =A0 =A0case 0x0fd4: > + =A0 =A0case 0x0fd5: > + =A0 =A0case 0x0fd6: > + =A0 =A0case 0x0fd7: > + =A0 =A0case 0x0fd8: > + =A0 =A0case 0x0fd9: > + =A0 =A0case 0x0fda: > + =A0 =A0case 0x0fdb: > + =A0 =A0case 0x0fdc: > + =A0 =A0case 0x0fdd: > + =A0 =A0case 0x0fde: > + =A0 =A0case 0x0fdf: > + =A0 =A0case 0x0fe0: > + =A0 =A0case 0x0fe1: > + =A0 =A0case 0x0fe2: > + =A0 =A0case 0x0fe3: > + =A0 =A0case 0x0fe4: > + =A0 =A0case 0x0fe5: > + =A0 =A0case 0x0fe6: > + =A0 =A0case 0x0fe7: > + =A0 =A0case 0x0fe8: > + =A0 =A0case 0x0fe9: > + =A0 =A0case 0x0fea: > + =A0 =A0case 0x0feb: > + =A0 =A0case 0x0fec: > + =A0 =A0case 0x0fed: > + =A0 =A0case 0x0fee: > + =A0 =A0case 0x0fef: > + =A0 =A0case 0x0ff0: > + =A0 =A0case 0x0ff1: > + =A0 =A0case 0x0ff2: > + =A0 =A0case 0x0ff3: > + =A0 =A0case 0x0ff4: > + =A0 =A0case 0x0ff5: > + =A0 =A0case 0x0ff6: > + =A0 =A0case 0x0ff7: > + =A0 =A0case 0x0ff8: > + =A0 =A0case 0x0ff9: > + =A0 =A0case 0x0ffa: > + =A0 =A0case 0x0ffb: > + =A0 =A0case 0x0ffc: > + =A0 =A0case 0x0ffd: > + =A0 =A0case 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