Re: [PATCH] Add vpermil2pd/vpermil2ps for AMD Orochi processor

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

On Wed, Feb 10, 2010 at 8:20 AM, Quentin Neill
<quentin.neill.gnu@gmail.com> wrote:
> On Wed, Feb 10, 2010 at 9:55 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Wed, Feb 10, 2010 at 7:38 AM, Quentin Neill
>> <quentin.neill.gnu@gmail.com> wrote:
>>> This patch restores vpermil2pd/vpermil2ps insns with 5 operand support.
>>>
>>> I started with the original Intel code removed back in
>>> ? ?http://sourceware.org/ml/binutils-cvs/2009-01/msg00025.html
>>> and then adjusted for the AMD implementation.
>>>
>>> Tested with make -k check.
>>>
>>> Okay to commit?
>>>
>>> gas:
>>> ? ? ? ?* config/tc-i386.c (vex_imm4) New operand type.
>>> ? ? ? ?(fits_in_imm4): New.
>>> ? ? ? ?(VEX_check_operands): New.
>>> ? ? ? ?(check_reverse): Call VEX_check_operands.
>>> ? ? ? ?(build_modrm_byte): Reintroduce code for 5
>>> ? ? ? ?operand insns. ?Fix whitespace.
>>>
>>> gas/testsuite:
>>> ? ? ? ?* gas/i386/x86-64-xop.d: Add vpermil2p[sd] tests.
>>> ? ? ? ?* testsuite/gas/i386/x86-64-xop.s: Likewise.
>>> ? ? ? ?* testsuite/gas/i386/xop.d: Likewise.
>>> ? ? ? ?* testsuite/gas/i386/xop.s: Likewise.
>>>
>>> opcodes:
>>> ? ? ? ?* i386-dis.c (OP_EX_VexImmW): Reintroduced
>>> ? ? ? ?function to handle 5th imm8 operand.
>>> ? ? ? ?(PREFIX_VEX_3A48): Added.
>>> ? ? ? ?(PREFIX_VEX_3A49): Added.
>>> ? ? ? ?(VEX_W_3A48_P_2): Added.
>>> ? ? ? ?(VEX_W_3A49_P_2): Added.
>>> ? ? ? ?(prefix table): Added entries for PREFIX_VEX_3A48
>>> ? ? ? ?and PREFIX_VEX_3A49.
>>> ? ? ? ?(vex table): Added entries for VEX_W_3A48_P_2 and
>>> ? ? ? ?and VEX_W_3A49_P_2.
>>> ? ? ? ?* i386-gen.c (operand_type_init): Added OPERAND_TYPE_VEX_IMM4
>>> ? ? ? ?for VEX_Imm4 operands.
>>> ? ? ? ?* i386-opc.h (enum): Added Vex_Imm4.
>>> ? ? ? ?(i386_operand_type): Added vex_imm4.
>>> ? ? ? ?* i386-opc.tbl: Add entries for vpermilp[ds].
>>> ? ? ? ?* i386-init.h: Regenerated.
>>> ? ? ? ?* i386-tbl.h: Regenerated.
>>>
>>
>> Please change VEX_Imm4 to Vec_Imm4.
>>
>> What is // in
>>
>> + ? ? ?gas_assert ((i.reg_operands == 4
>> + ? ? ? ? ? ? ? ? ? || (i.reg_operands == 3 && i.mem_operands == 1))
>> + ? ? ? ? ? ? ? ? ?&& i.tm.opcode_modifier.vexvvvv == VEXXDS
>> + ? ? ? ? ? ? ? ? ?&& i.tm.opcode_modifier.veximmext
>> + ? ? ? ? ? ? ? ? ?&& (operand_type_equal (&i.tm.operand_types[dest], &regxmm)
>> + ? ? ? ? ? ? ? ? ? ? ?|| operand_type_equal
>> (&i.tm.operand_types[dest], &regymm))
>> + ? ? ? ? ? ? ? ? ?&& ((dest == 4
>> + ? ? ? ? ? ? ? ? ? ? ? && i.imm_operands == 1
>> + ? ? ? ? ? ? ? ? ? ? ? //&& i.types[0].bitfield.veximmext
>> ? ? ? ? ? ? ? ? ? ? ? ^^^^^^^^^^^^^^^^^^^^^^^^^
>> + ? ? ? ? ? ? ? ? ? ? ? && (i.tm.opcode_modifier.vexw == VEXW0
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? || i.tm.opcode_modifier.vexw == VEXW1))
>> + ? ? ? ? ? ? ? ? ? ? ?|| (dest == 3
>> + ? ? ? ? ? ? ? ? ? ? ? ? ?&& (i.imm_operands == 0
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?|| (i.imm_operands == 1
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?&& i.tm.opcode_modifier.immext))
>> + ? ? ? ? ? ? ? ? ? ? ? ? ?&& i.tm.opcode_modifier.veximmext)));
>>
>> for?
>> --
>> H.J.
>
> Hi H.J.
>
> Thanks for the quick reply. ?You wrote:
>
>> Please change VEX_Imm4 to Vec_Imm4
>
> Did you mean "Vex_Imm4" (spelled with x, not c)?

I mean Vec_Imm4 with c not x. I will add other Vec_ImmX in the future.

>
>> + ? ? ? ? ? ? ? ? ? ? ? //&& i.types[0].bitfield.veximmext
>> ? ? ? ? ? ? ? ? ? ? ? ^^^^^^^^^^^^^^^^^^^^^^^^^
>
> As for the commented conditional, it was left unintentionally in the
> patch, but I am revisiting the logic in this gas_assert.
>

Please submit a new patch.

Thanks.


-- 
H.J.