Re: [PATCH] AMD bdver2 processors 1/2 - BMI

["H.J. Lu" <hjl dot tools at gmail dot com>]

On Tue, Jan 4, 2011 at 12:58 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Tue, Jan 4, 2011 at 12:23 PM, Quentin Neill
> <quentin.neill.gnu@gmail.com> wrote:
>> On Tue, Dec 28, 2010 at 8:10 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>> On Tue, Dec 28, 2010 at 5:53 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>> On Mon, Dec 20, 2010 at 2:55 PM, Quentin Neill
>>>> <quentin.neill.gnu@gmail.com> wrote:
>>>>> On Mon, Dec 20, 2010 at 4:39 PM, Sebastian Pop <sebpop@gmail.com> wrote:
>>>>>> On Mon, Dec 20, 2010 at 16:32, Quentin Neill
>>>>>> <quentin.neill.gnu@gmail.com> wrote:
>>>>>>> These two patches add support for BMI and TBM ISAs to be introduced in
>>>>>>> AMD bdver2 processors.
>>>>>>>
>>>>>>> The full encoding specification is delayed, however I have posted
>>>>>>> abbreviated specs on the gcc mailing list:
>>>>>>> BMI: http://gcc.gnu.org/ml/gcc-patches/2010-10/msg01766.html
>>>>>>> TBM: http://gcc.gnu.org/ml/gcc-patches/2010-10/msg01767.html
>>>>>>>
>>>>>>
>>>>>> Looks like your patch is reversed. ?Could you please send another one
>>>>>> that you get from git format-patch -1
>>>>>>
>>>>>> Thanks,
>>>>>> Sebastian
>>>>>
>>>>> Oops. ?Reposting with reversed patch. ?Thanks for reviewing Sebastian.
>>>>> --
>>>>> Quentin
>>>>>
>>>>
>>>> Please don't add ModrmRegExt, There are many examples in i386-opt.tbl
>>>> without ModrmRegExt.
>>>>
>>>>
>>>
>>> You should check i.tm.extension_opcode != None instead.
>>>
>>> --
>>> H.J.
>>
>> Fixed with the attached.
>> Tested and passes with "make check RUNTESTFLAGS=i386.exp".
>> Okay to commit?
>> --
>
> Comments on i386-dis.c:
>
> 1. For insns with VEX encoding, use XXX_VEX_0FXXXXX and sort them.
> 2. Use vex_len_table to handle invalid vector length.
> 3. Use VexGdq I just added instead of "{ OP_LWP_E, 0 }"
> 4. Properly add suffix with
>
> { "XXXS", ? ? ? ? ?{ Gdq, VexGdq, Edq } },
>
>

Your BMI implementation has some issues:

[hjl@gnu-6 i386]$ head -7 x86-64-bmi.s

	.allow_index_reg
	.text
	
_start:

    ANDN     %eax,%r15d,%eax
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[hjl@gnu-6 i386]$ head -9 x86-64-bmi.d
#objdump: -dw
#name: x86-64 BMI

.*: +file format .*

Disassembly of section .text:

0+ <_start>:
[ 	]*[a-f0-9]+:	c4 c2 00 f2 c0       	andn   %r8d,%r15d,%eax
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Your assembler doesn't match your disassembler. I checked in
my BMI implementation:

http://sourceware.org/ml/binutils/2011-01/msg00038.html

Please double check.

Thanks.

-- 
H.J.