[2/5] x86: re-order optimize_disp()

Message ID d627611d-ce44-a37a-0a17-91b4d66e9fce@suse.com
State New
Headers show
Series
  • x86: assorted relocation handling related adjustment (part I)
Related show

Commit Message

Jan Beulich via Binutils April 22, 2021, 8:39 a.m.
While I can't point out any specific case where things break, it looks
wrong to have the consumer of a flag before its producer. Set .disp32
first, then do the possible conversion to signed 32-bit, and finally
check whether the value fits in a signed long.

gas/
2021-04-XX  Jan Beulich  <jbeulich@suse.com>

	* config/tc-i386.c (optimize_disp): Move down BFD64 section.
	Move up setting of disp32.

Comments

Jan Beulich via Binutils April 22, 2021, 11:55 a.m. | #1
On Thu, Apr 22, 2021 at 1:39 AM Jan Beulich <jbeulich@suse.com> wrote:
>

> While I can't point out any specific case where things break, it looks

> wrong to have the consumer of a flag before its producer. Set .disp32

> first, then do the possible conversion to signed 32-bit, and finally

> check whether the value fits in a signed long.

>

> gas/

> 2021-04-XX  Jan Beulich  <jbeulich@suse.com>

>

>         * config/tc-i386.c (optimize_disp): Move down BFD64 section.

>         Move up setting of disp32.

>

> --- a/gas/config/tc-i386.c

> +++ b/gas/config/tc-i386.c

> @@ -5707,19 +5707,6 @@ optimize_disp (void)

>                 op_disp = (((op_disp & 0xffff) ^ 0x8000) - 0x8000);

>                 i.types[op].bitfield.disp64 = 0;

>               }

> -#ifdef BFD64

> -           /* Optimize 64-bit displacement to 32-bit for 64-bit BFD.  */

> -           if (i.types[op].bitfield.disp32

> -               && (op_disp & ~(((offsetT) 2 << 31) - 1)) == 0)

> -             {

> -               /* If this operand is at most 32 bits, convert

> -                  to a signed 32 bit number and don't use 64bit

> -                  displacement.  */

> -               op_disp &= (((offsetT) 2 << 31) - 1);

> -               op_disp = (op_disp ^ ((offsetT) 1 << 31)) - ((addressT) 1 << 31);

> -               i.types[op].bitfield.disp64 = 0;

> -             }

> -#endif

>             if (!op_disp && i.types[op].bitfield.baseindex)

>               {

>                 i.types[op].bitfield.disp8 = 0;

> @@ -5730,17 +5717,32 @@ optimize_disp (void)

>                 i.op[op].disps = 0;

>                 i.disp_operands--;

>               }

> +#ifdef BFD64

>             else if (flag_code == CODE_64BIT)

>               {

> +               if (i.prefix[ADDR_PREFIX]

> +                   && fits_in_unsigned_long (op_disp))

> +                 i.types[op].bitfield.disp32 = 1;

> +

> +               /* Optimize 64-bit displacement to 32-bit for 64-bit BFD.  */

> +               if (i.types[op].bitfield.disp32

> +                   && (op_disp & ~(((offsetT) 2 << 31) - 1)) == 0)

> +                 {

> +                   /* If this operand is at most 32 bits, convert

> +                      to a signed 32 bit number and don't use 64bit

> +                      displacement.  */

> +                   op_disp &= (((offsetT) 2 << 31) - 1);

> +                   op_disp = (op_disp ^ ((offsetT) 1 << 31)) - ((addressT) 1 << 31);

> +                   i.types[op].bitfield.disp64 = 0;

> +                 }

> +

>                 if (fits_in_signed_long (op_disp))

>                   {

>                     i.types[op].bitfield.disp64 = 0;

>                     i.types[op].bitfield.disp32s = 1;

>                   }

> -               if (i.prefix[ADDR_PREFIX]

> -                   && fits_in_unsigned_long (op_disp))

> -                 i.types[op].bitfield.disp32 = 1;

>               }

> +#endif

>             if ((i.types[op].bitfield.disp32

>                  || i.types[op].bitfield.disp32s

>                  || i.types[op].bitfield.disp16)

>


OK.

Thanks.

-- 
H.J.

Patch

--- a/gas/config/tc-i386.c
+++ b/gas/config/tc-i386.c
@@ -5707,19 +5707,6 @@  optimize_disp (void)
 		op_disp = (((op_disp & 0xffff) ^ 0x8000) - 0x8000);
 		i.types[op].bitfield.disp64 = 0;
 	      }
-#ifdef BFD64
-	    /* Optimize 64-bit displacement to 32-bit for 64-bit BFD.  */
-	    if (i.types[op].bitfield.disp32
-		&& (op_disp & ~(((offsetT) 2 << 31) - 1)) == 0)
-	      {
-		/* If this operand is at most 32 bits, convert
-		   to a signed 32 bit number and don't use 64bit
-		   displacement.  */
-		op_disp &= (((offsetT) 2 << 31) - 1);
-		op_disp = (op_disp ^ ((offsetT) 1 << 31)) - ((addressT) 1 << 31);
-		i.types[op].bitfield.disp64 = 0;
-	      }
-#endif
 	    if (!op_disp && i.types[op].bitfield.baseindex)
 	      {
 		i.types[op].bitfield.disp8 = 0;
@@ -5730,17 +5717,32 @@  optimize_disp (void)
 		i.op[op].disps = 0;
 		i.disp_operands--;
 	      }
+#ifdef BFD64
 	    else if (flag_code == CODE_64BIT)
 	      {
+		if (i.prefix[ADDR_PREFIX]
+		    && fits_in_unsigned_long (op_disp))
+		  i.types[op].bitfield.disp32 = 1;
+
+		/* Optimize 64-bit displacement to 32-bit for 64-bit BFD.  */
+		if (i.types[op].bitfield.disp32
+		    && (op_disp & ~(((offsetT) 2 << 31) - 1)) == 0)
+		  {
+		    /* If this operand is at most 32 bits, convert
+		       to a signed 32 bit number and don't use 64bit
+		       displacement.  */
+		    op_disp &= (((offsetT) 2 << 31) - 1);
+		    op_disp = (op_disp ^ ((offsetT) 1 << 31)) - ((addressT) 1 << 31);
+		    i.types[op].bitfield.disp64 = 0;
+		  }
+
 		if (fits_in_signed_long (op_disp))
 		  {
 		    i.types[op].bitfield.disp64 = 0;
 		    i.types[op].bitfield.disp32s = 1;
 		  }
-		if (i.prefix[ADDR_PREFIX]
-		    && fits_in_unsigned_long (op_disp))
-		  i.types[op].bitfield.disp32 = 1;
 	      }
+#endif
 	    if ((i.types[op].bitfield.disp32
 		 || i.types[op].bitfield.disp32s
 		 || i.types[op].bitfield.disp16)