RE: [PATCH] Add x86 SSE5 instructions to the GNU binary utilities

["rajagopal, dwarak" <dwarak dot rajagopal at amd dot com>]

I thought the restriction was 80 columns instead of 72.
Nevertheless, I have fixed all my changes to 72 columns. Also, I have
fixed all other comments you pointed out.

Ok?

Thanks,
Dwarak

> -----Original Message-----
> From: H.J. Lu [mailto:hjl@lucon.org]
> Sent: Friday, September 14, 2007 8:50 AM
> To: rajagopal, dwarak
> Cc: binutils@sourceware.org; Meissner, Michael; Harle, Christophe
> Subject: Re: [PATCH] Add x86 SSE5 instructions to the GNU binary
utilities
> 
> On Thu, Sep 13, 2007 at 09:51:00PM -0500, rajagopal, dwarak wrote:
> > Yes this fixes the issue. Thanks.
> > The enclosed patch now applies over your patch
> > (http://sourceware.org/ml/binutils/2007-09/msg00172.html).
> >
> 
> See my comments below.
> 
> 
> H.J.
> ----
> +  fprintf (stdout, "  drex:  reg %d rex 0x%x\n", x->drex.reg, x-
> >drex.rex);
> 
> Line too long
> 
> +  /* If the instruction has the DREX attribute (aka SSE5), don't emit
a
> +     REX prefix.  */
> +  if ((i.tm.opcode_modifier.drex || i.tm.opcode_modifier.drexc) != 0)
> 
> No need for () and != 0.
> 
> +  /* SSE5 4 operand instructions must have the destination the same
as
> one of
> 
> Line too long
> 
> +     the inputs.  Figure out the destination register and cache it
away
> in the
> 
> Line too long
> 
> +	       && i.types[1].bitfield.regxmm != 0
> +	       && (i.types[2].bitfield.regxmm
> +		   || operand_type_check (i.types[2], anymem)) != 0
> 
> No need for != 0.
> 
> +	  /* Specify the modrm encoding for memory addressing.  Include
the
> 
> Line too long
> 
> +	     high order bit that is normally stored in the REX byte in
the
> 
> Line too long
> 
> +      /* Case 5: 4 operand insn, dest = src3, src2 = memory.  */
> +      else if (i.types[0].bitfield.regxmm != 0
> +	       && (i.types[1].bitfield.regxmm
> +		   || operand_type_check (i.types[1], anymem)) != 0
> 
> No need for != 0. Please remove != 0 from all of
> 
> (A || B) != 0
> 
> in your patch.
> 
> +
> +	  /* Specify the modrm encoding and remember the register
including
> the
> 
> Line too long
> 
> +	  /* Specify the modrm encoding and remember the register
including
> the
> 
> Line too long. You have many long lines. Please make sure your change
> is less than 72 columns.
> 
> 
> +     encoded in the DREX byte. */
> +  else if (i.tm.opcode_modifier.drex && !i.tm.opcode_modifier.drexv
> +	   && i.operands == 4)
> 
> One line for each condition. Please double check other places for
> this.
> 
> +    {
> +      /* Case 1: 4 operand insn, dest = src1, src3 = reg/mem.  */
> +      if ((i.types[0].bitfield.regxmm) != 0
> 
> No need for ().
> 
> +	  && (i.types[1].bitfield.regxmm
> +	      || operand_type_check(i.types[1], anymem)) != 0
> 
> See above.
> 
> @@ -996,8 +1026,8 @@ static const struct dis386 dis386_twobyt
>    { OPC_EXT_41 },
>    { OPC_EXT_42 },
>    { OPC_EXT_43 },
> -  { OPC_EXT_44 },
> -  { "(bad)",		{ XX } },
> +  { OPC_EXT_44 }, /* also movL {Td, Rd} in 386/486 */
> 
> Don't change it.
> Keep
> 
>     { OPC_EXT_44 },
> 
> +  { THREE_BYTE_SSE5_0F25 },
>

Attachment: Changelog
Description: Changelog

Attachment: binutils-sse5.patch.gz
Description: binutils-sse5.patch.gz