public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [ARM] Allow MOV/MOV.W to accept all possible immediates
@ 2016-11-01 12:21 Jiong Wang
  2016-11-03 11:37 ` Nick Clifton
  2017-01-05 10:45 ` Szabolcs Nagy
  0 siblings, 2 replies; 8+ messages in thread
From: Jiong Wang @ 2016-11-01 12:21 UTC (permalink / raw)
  To: Binutils

[-- Attachment #1: Type: text/plain, Size: 1531 bytes --]

Hi,

   AArch32 MOV Rd, #imm should accept all immediates which can be encoded into
available encoding, A1/A2 for ARM, T1/T2/T3 for Thumb/Thumb2, and MOV is the
preferred disassembly syntax.  See latest ARM Architecture Reference Manual for
ARMv8-A, section F6.1.108 and for ARMv8-M, section C2.4.89.

   The same for MOV.W under Thumb mode.  It should try all possible 32-bit Thumb
encoding instead of T2 only.

   This patch let MOV/MOV.W accept more immediate formats while their currently
supported immediate formats are not affected, so there is no backward compatibility
issue, also this patch haven't touched the disassembler.

   I think this patch brings GAS closer to ARM Architecture Reference Manual.

   OK for master?

gas/
2016-11-01  Jiong Wang  <jiong.wang@arm.com>
                   
         * config/tc-arm.c (SBIT_SHIFT): New.
         (T2_SBIT_SHIFT): Likewise.
         (t32_insn_ok): Return TRUE for MOV in ARMv8-M Baseline.
         (md_apply_fix): Try UINT16 encoding when ARM/Thumb modified immediate
         encoding failed.
         * testsuite/gas/arm/archv6t2-bad.s: New error case.
         * testsuite/gas/arm/archv6t2-bad.l: New error match.
         * testsuite/gas/arm/archv6t2.s: New testcase.
         * testsuite/gas/arm/archv6t2.d: New expected result.
         * testsuite/gas/arm/archv8m.s: New testcase
         * testsuite/gas/arm/archv8m-base.d: New expected result.
         * testsuite/gas/arm/archv8m-main.d: Likewise.
         * testsuite/gas/arm/archv8m-main-dsp-1.d: Likewise.


[-- Attachment #2: fix-gas.patch --]
[-- Type: text/x-patch, Size: 9627 bytes --]

diff --git a/gas/config/tc-arm.c b/gas/config/tc-arm.c
index 73f8396396ff85276dc5bc92f79764d6d194b3d3..f28bf5297f33d69fd93961aaff51554434ab4223 100644
--- a/gas/config/tc-arm.c
+++ b/gas/config/tc-arm.c
@@ -685,9 +685,11 @@ struct asm_opcode
 #define T2_SUBS_PC_LR	0xf3de8f00
 
 #define DATA_OP_SHIFT	21
+#define SBIT_SHIFT	20
 
 #define T2_OPCODE_MASK	0xfe1fffff
 #define T2_DATA_OP_SHIFT 21
+#define T2_SBIT_SHIFT	 20
 
 #define A_COND_MASK         0xf0000000
 #define A_PUSH_POP_OP_MASK  0x0fff0000
@@ -18270,6 +18272,13 @@ t32_insn_ok (arm_feature_set arch, const struct asm_opcode *opcode)
       && opcode->tencode == do_t_branch)
     return TRUE;
 
+  /* MOV accepts T1/T3 encodings under Baseline, T3 encoding is 32bit.  */
+  if (ARM_CPU_HAS_FEATURE (arch, arm_ext_v8m)
+      && opcode->tencode == do_t_mov_cmp
+      /* Make sure CMP instruction is not affected.  */
+      && opcode->aencode == do_mov)
+    return TRUE;
+
   /* Wide instruction variants of all instructions with narrow *and* wide
      variants become available with ARMv6t2.  Other opcodes are either
      narrow-only or wide-only and are thus available if OPCODE is valid.  */
@@ -22773,6 +22782,23 @@ md_apply_fix (fixS *	fixP,
 	     changing the opcode.  */
 	  if (newimm == (unsigned int) FAIL)
 	    newimm = negate_data_op (&temp, value);
+	  /* MOV accepts both ARM modified immediate (A1 encoding) and
+	     UINT16 (A2 encoding) when possible, MOVW only accepts UINT16.
+	     When disassembling, MOV is preferred when there is no encoding
+	     overlap.  */
+	  if (newimm == (unsigned int) FAIL
+	      && ((temp >> DATA_OP_SHIFT) & 0xf) == OPCODE_MOV
+	      && ARM_CPU_HAS_FEATURE (cpu_variant, arm_ext_v6t2)
+	      && !((temp >> SBIT_SHIFT) & 0x1)
+	      && value >= 0 && value <= 0xffff)
+	    {
+	      /* Clear bits[23:20] to change encoding from A1 to A2.  */
+	      temp &= 0xff0fffff;
+	      /* Encoding high 4bits imm.  Code below will encode the remaining
+		 low 12bits.  */
+	      temp |= (value & 0x0000f000) << 4;
+	      newimm = value & 0x00000fff;
+	    }
 	}
 
       if (newimm == (unsigned int) FAIL)
@@ -23089,32 +23115,59 @@ md_apply_fix (fixS *	fixP,
       newval |= md_chars_to_number (buf+2, THUMB_SIZE);
 
       newimm = FAIL;
-      if (fixP->fx_r_type == BFD_RELOC_ARM_T32_IMMEDIATE
+      if ((fixP->fx_r_type == BFD_RELOC_ARM_T32_IMMEDIATE
+	   /* ARMv8-M Baseline MOV will reach here, but it doesn't support
+	      Thumb2 modified immediate encoding (T2).  */
+	   && ARM_CPU_HAS_FEATURE (cpu_variant, arm_ext_v6t2))
 	  || fixP->fx_r_type == BFD_RELOC_ARM_T32_ADD_IMM)
 	{
 	  newimm = encode_thumb32_immediate (value);
 	  if (newimm == (unsigned int) FAIL)
 	    newimm = thumb32_negate_data_op (&newval, value);
 	}
-      if (fixP->fx_r_type != BFD_RELOC_ARM_T32_IMMEDIATE
-	  && newimm == (unsigned int) FAIL)
+      if (newimm == (unsigned int) FAIL)
 	{
-	  /* Turn add/sum into addw/subw.  */
-	  if (fixP->fx_r_type == BFD_RELOC_ARM_T32_ADD_IMM)
-	    newval = (newval & 0xfeffffff) | 0x02000000;
-	  /* No flat 12-bit imm encoding for addsw/subsw.  */
-	  if ((newval & 0x00100000) == 0)
+	  if (fixP->fx_r_type != BFD_RELOC_ARM_T32_IMMEDIATE)
 	    {
-	      /* 12 bit immediate for addw/subw.  */
-	      if (value < 0)
+	      /* Turn add/sum into addw/subw.  */
+	      if (fixP->fx_r_type == BFD_RELOC_ARM_T32_ADD_IMM)
+		newval = (newval & 0xfeffffff) | 0x02000000;
+	      /* No flat 12-bit imm encoding for addsw/subsw.  */
+	      if ((newval & 0x00100000) == 0)
 		{
-		  value = -value;
-		  newval ^= 0x00a00000;
+		  /* 12 bit immediate for addw/subw.  */
+		  if (value < 0)
+		    {
+		      value = -value;
+		      newval ^= 0x00a00000;
+		    }
+		  if (value > 0xfff)
+		    newimm = (unsigned int) FAIL;
+		  else
+		    newimm = value;
+		}
+	    }
+	  else
+	    {
+	      /* MOV accepts both Thumb2 modified immediate (T2 encoding) and
+		 UINT16 (T3 encoding), MOVW only accepts UINT16.  When
+		 disassembling, MOV is preferred when there is no encoding
+		 overlap.
+		 NOTE: MOV is using ORR opcode under Thumb 2 mode.  */
+	      if (((newval >> T2_DATA_OP_SHIFT) & 0xf) == T2_OPCODE_ORR
+		  && ARM_CPU_HAS_FEATURE (cpu_variant, arm_ext_v6t2_v8m)
+		  && !((newval >> T2_SBIT_SHIFT) & 0x1)
+		  && value >= 0 && value <=0xffff)
+		{
+		  /* Toggle bit[25] to change encoding from T2 to T3.  */
+		  newval ^= 1 << 25;
+		  /* Clear bits[19:16].  */
+		  newval &= 0xfff0ffff;
+		  /* Encoding high 4bits imm.  Code below will encode the
+		     remaining low 12bits.  */
+		  newval |= (value & 0x0000f000) << 4;
+		  newimm = value & 0x00000fff;
 		}
-	      if (value > 0xfff)
-		newimm = (unsigned int) FAIL;
-	      else
-		newimm = value;
 	    }
 	}
 
diff --git a/gas/testsuite/gas/arm/archv6t2-bad.l b/gas/testsuite/gas/arm/archv6t2-bad.l
index 0f00db37b2b826c1a55f194d16b455c7c7421a26..89c28ecd2314c6405eb646f8b29ceeb30f066896 100644
--- a/gas/testsuite/gas/arm/archv6t2-bad.l
+++ b/gas/testsuite/gas/arm/archv6t2-bad.l
@@ -38,3 +38,4 @@
 [^:]*:53: Warning: source register same as write-back base
 [^:]*:59: Error: instruction does not accept this addressing mode -- `ldrex r0,r2'
 [^:]*:60: Error: instruction does not accept this addressing mode -- `strex r1,r0,r2'
+[^:]*:64: Error: invalid constant \(999\) after fixup
diff --git a/gas/testsuite/gas/arm/archv6t2-bad.s b/gas/testsuite/gas/arm/archv6t2-bad.s
index af1397271b629b0293d728ba1ebd6e5d656b767d..008088d923b6bb29bce440f04c6720735fabedd5 100644
--- a/gas/testsuite/gas/arm/archv6t2-bad.s
+++ b/gas/testsuite/gas/arm/archv6t2-bad.s
@@ -58,4 +58,7 @@ x:
 	@ to a label called "r2".
 	ldrex	r0, r2
 	strex	r1, r0, r2
-	
\ No newline at end of file
+
+	@ movs shouldn't be extened to accept UINT16 can't fit into ARM modified
+	@ immediate format.
+	movs	r0, #0x0999
diff --git a/gas/testsuite/gas/arm/archv6t2.d b/gas/testsuite/gas/arm/archv6t2.d
index eb76a321a8edfee66b8676cb7b909ac0b3cf4b12..8769b3f3a180f7bb256950b12fc11f0d9e7c2a92 100644
--- a/gas/testsuite/gas/arm/archv6t2.d
+++ b/gas/testsuite/gas/arm/archv6t2.d
@@ -61,3 +61,4 @@ Disassembly of section .text:
 0+d4 <[^>]+> e06100b2 	strht	r0, \[r1\], #-2
 0+d8 <[^>]+> 10e100b2 	strhtne	r0, \[r1\], #2
 0+dc <[^>]+> 106100b2 	strhtne	r0, \[r1\], #-2
+0+e0 <[^>]+> e3009999 	movw	r9, #2457	; 0x999
diff --git a/gas/testsuite/gas/arm/archv6t2.s b/gas/testsuite/gas/arm/archv6t2.s
index 81ff5012f5dec6f01cb657ea9e212e9ee07cf1b6..8d74359ad7b430d97af9f7c298198a416033f6b8 100644
--- a/gas/testsuite/gas/arm/archv6t2.s
+++ b/gas/testsuite/gas/arm/archv6t2.s
@@ -65,3 +65,6 @@ x:
 	strht	r0, [r1], #-2
 	strneht	r0, [r1], #2
 	strneht	r0, [r1], #-2
+
+	@ mov accept A2 encoding as well.
+	mov	r9, #0x0999
diff --git a/gas/testsuite/gas/arm/archv8m-base.d b/gas/testsuite/gas/arm/archv8m-base.d
index 6a2ee87f3b6eb90754c91c78788276b2bfd92880..6075ee048a077d0a6127607ad7d05fcccaa03cff 100644
--- a/gas/testsuite/gas/arm/archv8m-base.d
+++ b/gas/testsuite/gas/arm/archv8m-base.d
@@ -16,6 +16,8 @@ Disassembly of section .text:
 0+.* <[^>]*> e849 f840 	ttt	r8, r9
 0+.* <[^>]*> f24f 1023 	movw	r0, #61731	; 0xf123
 0+.* <[^>]*> f24f 1823 	movw	r8, #61731	; 0xf123
+0+.* <[^>]*> f24f 1823 	movw	r8, #61731	; 0xf123
+0+.* <[^>]*> f24f 1823 	movw	r8, #61731	; 0xf123
 0+.* <[^>]*> f2cf 1023 	movt	r0, #61731	; 0xf123
 0+.* <[^>]*> f2cf 1823 	movt	r8, #61731	; 0xf123
 0+.* <[^>]*> b154      	cbz	r4, 0+.* <[^>]*>
diff --git a/gas/testsuite/gas/arm/archv8m-main-dsp-1.d b/gas/testsuite/gas/arm/archv8m-main-dsp-1.d
index c8f9d7b81bf1af3949afffa7d4d3df7f3065f4bc..8c2c12d0d1cac7ca1b2f8de9abe92aa0802efc81 100644
--- a/gas/testsuite/gas/arm/archv8m-main-dsp-1.d
+++ b/gas/testsuite/gas/arm/archv8m-main-dsp-1.d
@@ -16,6 +16,8 @@ Disassembly of section .text:
 0+.* <[^>]*> e849 f840 	ttt	r8, r9
 0+.* <[^>]*> f24f 1023 	movw	r0, #61731	; 0xf123
 0+.* <[^>]*> f24f 1823 	movw	r8, #61731	; 0xf123
+0+.* <[^>]*> f24f 1823 	movw	r8, #61731	; 0xf123
+0+.* <[^>]*> f24f 1823 	movw	r8, #61731	; 0xf123
 0+.* <[^>]*> f2cf 1023 	movt	r0, #61731	; 0xf123
 0+.* <[^>]*> f2cf 1823 	movt	r8, #61731	; 0xf123
 0+.* <[^>]*> b154      	cbz	r4, 0+.* <[^>]*>
diff --git a/gas/testsuite/gas/arm/archv8m-main.d b/gas/testsuite/gas/arm/archv8m-main.d
index a0c40e99c89cef9aaef41c252bc37806d776f278..0b76db10fbda6c972007bb62ff303401f12e506a 100644
--- a/gas/testsuite/gas/arm/archv8m-main.d
+++ b/gas/testsuite/gas/arm/archv8m-main.d
@@ -16,6 +16,8 @@ Disassembly of section .text:
 0+.* <[^>]*> e849 f840 	ttt	r8, r9
 0+.* <[^>]*> f24f 1023 	movw	r0, #61731	; 0xf123
 0+.* <[^>]*> f24f 1823 	movw	r8, #61731	; 0xf123
+0+.* <[^>]*> f24f 1823 	movw	r8, #61731	; 0xf123
+0+.* <[^>]*> f24f 1823 	movw	r8, #61731	; 0xf123
 0+.* <[^>]*> f2cf 1023 	movt	r0, #61731	; 0xf123
 0+.* <[^>]*> f2cf 1823 	movt	r8, #61731	; 0xf123
 0+.* <[^>]*> b154      	cbz	r4, 0+.* <[^>]*>
diff --git a/gas/testsuite/gas/arm/archv8m.s b/gas/testsuite/gas/arm/archv8m.s
index 5f8aafed48746e19056a33405104a9768ed9feab..a82582eb889420177c5ab2afa0e4b555ec6b2047 100644
--- a/gas/testsuite/gas/arm/archv8m.s
+++ b/gas/testsuite/gas/arm/archv8m.s
@@ -11,6 +11,14 @@ tt    r8, r9
 ttt   r0, r1
 ttt   r8, r9
 movw  r0, #0xF123
+@ mov accept all immediate formats, including T3.  It's also the suggested
+@ assembly to use.
+mov   r8, #0xF123
+@ .w means wide, specifies that the assembler must select a 32-bit encoding for
+@ the instruction if it is possible, it should accept both T2 (Thumb modified
+@ immediate) and T3 (UINT16) encoding.  See the section "Standard assembler
+@ syntax fields" on latest ARM-ARM.
+mov.w r8, #0xF123
 movw  r8, #0xF123
 movt  r0, #0xF123
 movt  r8, #0xF123

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [ARM] Allow MOV/MOV.W to accept all possible immediates
  2016-11-01 12:21 [ARM] Allow MOV/MOV.W to accept all possible immediates Jiong Wang
@ 2016-11-03 11:37 ` Nick Clifton
  2017-01-05 10:45 ` Szabolcs Nagy
  1 sibling, 0 replies; 8+ messages in thread
From: Nick Clifton @ 2016-11-03 11:37 UTC (permalink / raw)
  To: Jiong Wang, Binutils

Hi Jiong,

> gas/
> 2016-11-01  Jiong Wang  <jiong.wang@arm.com>
>                           * config/tc-arm.c (SBIT_SHIFT): New.
>         (T2_SBIT_SHIFT): Likewise.
>         (t32_insn_ok): Return TRUE for MOV in ARMv8-M Baseline.
>         (md_apply_fix): Try UINT16 encoding when ARM/Thumb modified immediate
>         encoding failed.
>         * testsuite/gas/arm/archv6t2-bad.s: New error case.
>         * testsuite/gas/arm/archv6t2-bad.l: New error match.
>         * testsuite/gas/arm/archv6t2.s: New testcase.
>         * testsuite/gas/arm/archv6t2.d: New expected result.
>         * testsuite/gas/arm/archv8m.s: New testcase
>         * testsuite/gas/arm/archv8m-base.d: New expected result.
>         * testsuite/gas/arm/archv8m-main.d: Likewise.
>         * testsuite/gas/arm/archv8m-main-dsp-1.d: Likewise.

Approved - please apply.

Cheers
  Nick

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [ARM] Allow MOV/MOV.W to accept all possible immediates
  2016-11-01 12:21 [ARM] Allow MOV/MOV.W to accept all possible immediates Jiong Wang
  2016-11-03 11:37 ` Nick Clifton
@ 2017-01-05 10:45 ` Szabolcs Nagy
  2017-01-05 10:49   ` Szabolcs Nagy
  1 sibling, 1 reply; 8+ messages in thread
From: Szabolcs Nagy @ 2017-01-05 10:45 UTC (permalink / raw)
  To: Jiong Wang, Binutils; +Cc: nd

On 01/11/16 12:21, Jiong Wang wrote:
> Hi,
> 
>   AArch32 MOV Rd, #imm should accept all immediates which can be encoded into
> available encoding, A1/A2 for ARM, T1/T2/T3 for Thumb/Thumb2, and MOV is the
> preferred disassembly syntax.  See latest ARM Architecture Reference Manual for
> ARMv8-A, section F6.1.108 and for ARMv8-M, section C2.4.89.
> 
>   The same for MOV.W under Thumb mode.  It should try all possible 32-bit Thumb
> encoding instead of T2 only.
> 
>   This patch let MOV/MOV.W accept more immediate formats while their currently
> supported immediate formats are not affected, so there is no backward compatibility
> issue, also this patch haven't touched the disassembler.
> 
>   I think this patch brings GAS closer to ARM Architecture Reference Manual.
> 
>   OK for master?
> 
> gas/
> 2016-11-01  Jiong Wang  <jiong.wang@arm.com>
>                           * config/tc-arm.c (SBIT_SHIFT): New.
>         (T2_SBIT_SHIFT): Likewise.
>         (t32_insn_ok): Return TRUE for MOV in ARMv8-M Baseline.
>         (md_apply_fix): Try UINT16 encoding when ARM/Thumb modified immediate
>         encoding failed.
>         * testsuite/gas/arm/archv6t2-bad.s: New error case.
>         * testsuite/gas/arm/archv6t2-bad.l: New error match.
>         * testsuite/gas/arm/archv6t2.s: New testcase.
>         * testsuite/gas/arm/archv6t2.d: New expected result.
>         * testsuite/gas/arm/archv8m.s: New testcase
>         * testsuite/gas/arm/archv8m-base.d: New expected result.
>         * testsuite/gas/arm/archv8m-main.d: Likewise.
>         * testsuite/gas/arm/archv8m-main-dsp-1.d: Likewise.

this caused

$ cat bug.s
.syntax unified
.text
.arch armv7-a
	mov r3,#1

.arch armv4t
.eabi_attribute 6,2
	bx lr

$ arm-none-linux-gnueabihf-as -mthumb bug.s
bug.s: Assembler messages:
bug.s:4: Error: invalid constant (1) after fixup

previously this worked, but i'm not sure if the mix
of .arch directives and .syntax unified is valid here.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [ARM] Allow MOV/MOV.W to accept all possible immediates
  2017-01-05 10:45 ` Szabolcs Nagy
@ 2017-01-05 10:49   ` Szabolcs Nagy
  2017-01-05 13:59     ` Jiong Wang
  0 siblings, 1 reply; 8+ messages in thread
From: Szabolcs Nagy @ 2017-01-05 10:49 UTC (permalink / raw)
  To: Jiong Wang, Binutils; +Cc: nd, Rich Felker, Reiner Herrmann

adding the reporter to cc and rich because it broke musl.

On 05/01/17 10:45, Szabolcs Nagy wrote:
> On 01/11/16 12:21, Jiong Wang wrote:
>> Hi,
>>
>>   AArch32 MOV Rd, #imm should accept all immediates which can be encoded into
>> available encoding, A1/A2 for ARM, T1/T2/T3 for Thumb/Thumb2, and MOV is the
>> preferred disassembly syntax.  See latest ARM Architecture Reference Manual for
>> ARMv8-A, section F6.1.108 and for ARMv8-M, section C2.4.89.
>>
>>   The same for MOV.W under Thumb mode.  It should try all possible 32-bit Thumb
>> encoding instead of T2 only.
>>
>>   This patch let MOV/MOV.W accept more immediate formats while their currently
>> supported immediate formats are not affected, so there is no backward compatibility
>> issue, also this patch haven't touched the disassembler.
>>
>>   I think this patch brings GAS closer to ARM Architecture Reference Manual.
>>
>>   OK for master?
>>
>> gas/
>> 2016-11-01  Jiong Wang  <jiong.wang@arm.com>
>>                           * config/tc-arm.c (SBIT_SHIFT): New.
>>         (T2_SBIT_SHIFT): Likewise.
>>         (t32_insn_ok): Return TRUE for MOV in ARMv8-M Baseline.
>>         (md_apply_fix): Try UINT16 encoding when ARM/Thumb modified immediate
>>         encoding failed.
>>         * testsuite/gas/arm/archv6t2-bad.s: New error case.
>>         * testsuite/gas/arm/archv6t2-bad.l: New error match.
>>         * testsuite/gas/arm/archv6t2.s: New testcase.
>>         * testsuite/gas/arm/archv6t2.d: New expected result.
>>         * testsuite/gas/arm/archv8m.s: New testcase
>>         * testsuite/gas/arm/archv8m-base.d: New expected result.
>>         * testsuite/gas/arm/archv8m-main.d: Likewise.
>>         * testsuite/gas/arm/archv8m-main-dsp-1.d: Likewise.
> 
> this caused
> 
> $ cat bug.s
> .syntax unified
> .text
> .arch armv7-a
> 	mov r3,#1
> 
> .arch armv4t
> .eabi_attribute 6,2
> 	bx lr
> 
> $ arm-none-linux-gnueabihf-as -mthumb bug.s
> bug.s: Assembler messages:
> bug.s:4: Error: invalid constant (1) after fixup
> 
> previously this worked, but i'm not sure if the mix
> of .arch directives and .syntax unified is valid here.
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [ARM] Allow MOV/MOV.W to accept all possible immediates
  2017-01-05 10:49   ` Szabolcs Nagy
@ 2017-01-05 13:59     ` Jiong Wang
  2017-01-05 14:16       ` Jiong Wang
  2017-01-05 14:22       ` Jan Beulich
  0 siblings, 2 replies; 8+ messages in thread
From: Jiong Wang @ 2017-01-05 13:59 UTC (permalink / raw)
  To: Szabolcs Nagy; +Cc: Binutils, Rich Felker, Reiner Herrmann

On 05/01/17 10:49, Szabolcs Nagy wrote:
> adding the reporter to cc and rich because it broke musl.
>
> On 05/01/17 10:45, Szabolcs Nagy wrote:
>> On 01/11/16 12:21, Jiong Wang wrote:
>>> Hi,
>>>
>>>    AArch32 MOV Rd, #imm should accept all immediates which can be encoded into
>>> available encoding, A1/A2 for ARM, T1/T2/T3 for Thumb/Thumb2, and MOV is the
>>> preferred disassembly syntax.  See latest ARM Architecture Reference Manual for
>>> ARMv8-A, section F6.1.108 and for ARMv8-M, section C2.4.89.
>>>
>>>    The same for MOV.W under Thumb mode.  It should try all possible 32-bit Thumb
>>> encoding instead of T2 only.
>>>
>>>    This patch let MOV/MOV.W accept more immediate formats while their currently
>>> supported immediate formats are not affected, so there is no backward compatibility
>>> issue, also this patch haven't touched the disassembler.
>>>
>>>    I think this patch brings GAS closer to ARM Architecture Reference Manual.
>>>
>>>    OK for master?
>>>
>>> gas/
>>> 2016-11-01  Jiong Wang  <jiong.wang@arm.com>
>>>                            * config/tc-arm.c (SBIT_SHIFT): New.
>>>          (T2_SBIT_SHIFT): Likewise.
>>>          (t32_insn_ok): Return TRUE for MOV in ARMv8-M Baseline.
>>>          (md_apply_fix): Try UINT16 encoding when ARM/Thumb modified immediate
>>>          encoding failed.
>>>          * testsuite/gas/arm/archv6t2-bad.s: New error case.
>>>          * testsuite/gas/arm/archv6t2-bad.l: New error match.
>>>          * testsuite/gas/arm/archv6t2.s: New testcase.
>>>          * testsuite/gas/arm/archv6t2.d: New expected result.
>>>          * testsuite/gas/arm/archv8m.s: New testcase
>>>          * testsuite/gas/arm/archv8m-base.d: New expected result.
>>>          * testsuite/gas/arm/archv8m-main.d: Likewise.
>>>          * testsuite/gas/arm/archv8m-main-dsp-1.d: Likewise.
>> this caused
>>
>> $ cat bug.s
>> .syntax unified
>> .text
>> .arch armv7-a
>> 	mov r3,#1
>>
>> .arch armv4t
>> .eabi_attribute 6,2
>> 	bx lr
>>
>> $ arm-none-linux-gnueabihf-as -mthumb bug.s
>> bug.s: Assembler messages:
>> bug.s:4: Error: invalid constant (1) after fixup
>>
>> previously this worked, but i'm not sure if the mix
>> of .arch directives and .syntax unified is valid here.

It looks to me the failure is caused by the second ".arch armv4t" 
overrides the
first ".arch armv7-a", therefore the arch feature is lowerd to armv4t in
assembler fixup stage.  While "mov r3, #1" under thumb mode is only allowed
with ARMv6T2.

This patch tightend the check from

       if (fixP->fx_r_type == BFD_RELOC_ARM_T32_IMMEDIATE
           || fixP->fx_r_type == BFD_RELOC_ARM_T32_ADD_IMM)

into:

       if ((fixP->fx_r_type == BFD_RELOC_ARM_T32_IMMEDIATE
            && ARM_CPU_HAS_FEATURE (cpu_variant, arm_ext_v6t2))
           || fixP->fx_r_type == BFD_RELOC_ARM_T32_ADD_IMM)


thus the sequences in bug.s is not allowed.


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [ARM] Allow MOV/MOV.W to accept all possible immediates
  2017-01-05 13:59     ` Jiong Wang
@ 2017-01-05 14:16       ` Jiong Wang
  2017-02-17 14:55         ` Szabolcs Nagy
  2017-01-05 14:22       ` Jan Beulich
  1 sibling, 1 reply; 8+ messages in thread
From: Jiong Wang @ 2017-01-05 14:16 UTC (permalink / raw)
  To: Szabolcs Nagy; +Cc: Binutils, Rich Felker, Reiner Herrmann

On 05/01/17 13:59, Jiong Wang wrote:
> On 05/01/17 10:49, Szabolcs Nagy wrote:
>> adding the reporter to cc and rich because it broke musl.
>>
>> On 05/01/17 10:45, Szabolcs Nagy wrote:
>>> On 01/11/16 12:21, Jiong Wang wrote:
>>>> Hi,
>>>>
>>>>    AArch32 MOV Rd, #imm should accept all immediates which can be 
>>>> encoded into
>>>> available encoding, A1/A2 for ARM, T1/T2/T3 for Thumb/Thumb2, and 
>>>> MOV is the
>>>> preferred disassembly syntax.  See latest ARM Architecture 
>>>> Reference Manual for
>>>> ARMv8-A, section F6.1.108 and for ARMv8-M, section C2.4.89.
>>>>
>>>>    The same for MOV.W under Thumb mode.  It should try all possible 
>>>> 32-bit Thumb
>>>> encoding instead of T2 only.
>>>>
>>>>    This patch let MOV/MOV.W accept more immediate formats while 
>>>> their currently
>>>> supported immediate formats are not affected, so there is no 
>>>> backward compatibility
>>>> issue, also this patch haven't touched the disassembler.
>>>>
>>>>    I think this patch brings GAS closer to ARM Architecture 
>>>> Reference Manual.
>>>>
>>>>    OK for master?
>>>>
>>>> gas/
>>>> 2016-11-01  Jiong Wang  <jiong.wang@arm.com>
>>>>                            * config/tc-arm.c (SBIT_SHIFT): New.
>>>>          (T2_SBIT_SHIFT): Likewise.
>>>>          (t32_insn_ok): Return TRUE for MOV in ARMv8-M Baseline.
>>>>          (md_apply_fix): Try UINT16 encoding when ARM/Thumb 
>>>> modified immediate
>>>>          encoding failed.
>>>>          * testsuite/gas/arm/archv6t2-bad.s: New error case.
>>>>          * testsuite/gas/arm/archv6t2-bad.l: New error match.
>>>>          * testsuite/gas/arm/archv6t2.s: New testcase.
>>>>          * testsuite/gas/arm/archv6t2.d: New expected result.
>>>>          * testsuite/gas/arm/archv8m.s: New testcase
>>>>          * testsuite/gas/arm/archv8m-base.d: New expected result.
>>>>          * testsuite/gas/arm/archv8m-main.d: Likewise.
>>>>          * testsuite/gas/arm/archv8m-main-dsp-1.d: Likewise.
>>> this caused
>>>
>>> $ cat bug.s
>>> .syntax unified
>>> .text
>>> .arch armv7-a
>>>     mov r3,#1
>>>
>>> .arch armv4t
>>> .eabi_attribute 6,2
>>>     bx lr
>>>
>>> $ arm-none-linux-gnueabihf-as -mthumb bug.s
>>> bug.s: Assembler messages:
>>> bug.s:4: Error: invalid constant (1) after fixup
>>>
>>> previously this worked, but i'm not sure if the mix
>>> of .arch directives and .syntax unified is valid here.
>
> It looks to me the failure is caused by the second ".arch armv4t" 
> overrides the
> first ".arch armv7-a", therefore the arch feature is lowerd to armv4t in
> assembler fixup stage.  While "mov r3, #1" under thumb mode is only 
> allowed
> with ARMv6T2.
>
> This patch tightend the check from
>
>       if (fixP->fx_r_type == BFD_RELOC_ARM_T32_IMMEDIATE
>           || fixP->fx_r_type == BFD_RELOC_ARM_T32_ADD_IMM)
>
> into:
>
>       if ((fixP->fx_r_type == BFD_RELOC_ARM_T32_IMMEDIATE
>            && ARM_CPU_HAS_FEATURE (cpu_variant, arm_ext_v6t2))
>           || fixP->fx_r_type == BFD_RELOC_ARM_T32_ADD_IMM)
>
>
> thus the sequences in bug.s is not allowed.

Does .object_arch works for you? It seems you want the final object arch 
adjusted.

https://sourceware.org/ml/binutils/2006-09/msg00054.html

>
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [ARM] Allow MOV/MOV.W to accept all possible immediates
  2017-01-05 13:59     ` Jiong Wang
  2017-01-05 14:16       ` Jiong Wang
@ 2017-01-05 14:22       ` Jan Beulich
  1 sibling, 0 replies; 8+ messages in thread
From: Jan Beulich @ 2017-01-05 14:22 UTC (permalink / raw)
  To: Jiong Wang; +Cc: Szabolcs Nagy, Rich Felker, Reiner Herrmann, Binutils

>>> On 05.01.17 at 14:59, <jiong.wang@foss.arm.com> wrote:
> On 05/01/17 10:49, Szabolcs Nagy wrote:
>> On 05/01/17 10:45, Szabolcs Nagy wrote:
>>> $ cat bug.s
>>> .syntax unified
>>> .text
>>> .arch armv7-a
>>> 	mov r3,#1
>>>
>>> .arch armv4t
>>> .eabi_attribute 6,2
>>> 	bx lr
>>>
>>> $ arm-none-linux-gnueabihf-as -mthumb bug.s
>>> bug.s: Assembler messages:
>>> bug.s:4: Error: invalid constant (1) after fixup
>>>
>>> previously this worked, but i'm not sure if the mix
>>> of .arch directives and .syntax unified is valid here.
> 
> It looks to me the failure is caused by the second ".arch armv4t" 
> overrides the
> first ".arch armv7-a", therefore the arch feature is lowerd to armv4t in
> assembler fixup stage.  While "mov r3, #1" under thumb mode is only allowed
> with ARMv6T2.
> 
> This patch tightend the check from
> 
>        if (fixP->fx_r_type == BFD_RELOC_ARM_T32_IMMEDIATE
>            || fixP->fx_r_type == BFD_RELOC_ARM_T32_ADD_IMM)
> 
> into:
> 
>        if ((fixP->fx_r_type == BFD_RELOC_ARM_T32_IMMEDIATE
>             && ARM_CPU_HAS_FEATURE (cpu_variant, arm_ext_v6t2))
>            || fixP->fx_r_type == BFD_RELOC_ARM_T32_ADD_IMM)
> 
> 
> thus the sequences in bug.s is not allowed.

Suggests pretty clearly that the fixup should record the arch in
use at the time it is being created.

Jan

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [ARM] Allow MOV/MOV.W to accept all possible immediates
  2017-01-05 14:16       ` Jiong Wang
@ 2017-02-17 14:55         ` Szabolcs Nagy
  0 siblings, 0 replies; 8+ messages in thread
From: Szabolcs Nagy @ 2017-02-17 14:55 UTC (permalink / raw)
  To: Jiong Wang; +Cc: nd, Binutils, Rich Felker, Reiner Herrmann

On 05/01/17 14:16, Jiong Wang wrote:
> On 05/01/17 13:59, Jiong Wang wrote:
>> On 05/01/17 10:49, Szabolcs Nagy wrote:
>>> adding the reporter to cc and rich because it broke musl.
>>>
>>> On 05/01/17 10:45, Szabolcs Nagy wrote:
>>>> On 01/11/16 12:21, Jiong Wang wrote:
>>>>> Hi,
>>>>>
>>>>>    AArch32 MOV Rd, #imm should accept all immediates which can be encoded into
>>>>> available encoding, A1/A2 for ARM, T1/T2/T3 for Thumb/Thumb2, and MOV is the
>>>>> preferred disassembly syntax.  See latest ARM Architecture Reference Manual for
>>>>> ARMv8-A, section F6.1.108 and for ARMv8-M, section C2.4.89.
>>>>>
>>>>>    The same for MOV.W under Thumb mode.  It should try all possible 32-bit Thumb
>>>>> encoding instead of T2 only.
>>>>>
>>>>>    This patch let MOV/MOV.W accept more immediate formats while their currently
>>>>> supported immediate formats are not affected, so there is no backward compatibility
>>>>> issue, also this patch haven't touched the disassembler.
>>>>>
>>>>>    I think this patch brings GAS closer to ARM Architecture Reference Manual.
>>>>>
>>>>>    OK for master?
>>>>>
>>>>> gas/
>>>>> 2016-11-01  Jiong Wang  <jiong.wang@arm.com>
>>>>>                            * config/tc-arm.c (SBIT_SHIFT): New.
>>>>>          (T2_SBIT_SHIFT): Likewise.
>>>>>          (t32_insn_ok): Return TRUE for MOV in ARMv8-M Baseline.
>>>>>          (md_apply_fix): Try UINT16 encoding when ARM/Thumb modified immediate
>>>>>          encoding failed.
>>>>>          * testsuite/gas/arm/archv6t2-bad.s: New error case.
>>>>>          * testsuite/gas/arm/archv6t2-bad.l: New error match.
>>>>>          * testsuite/gas/arm/archv6t2.s: New testcase.
>>>>>          * testsuite/gas/arm/archv6t2.d: New expected result.
>>>>>          * testsuite/gas/arm/archv8m.s: New testcase
>>>>>          * testsuite/gas/arm/archv8m-base.d: New expected result.
>>>>>          * testsuite/gas/arm/archv8m-main.d: Likewise.
>>>>>          * testsuite/gas/arm/archv8m-main-dsp-1.d: Likewise.
>>>> this caused
>>>>
>>>> $ cat bug.s
>>>> .syntax unified
>>>> .text
>>>> .arch armv7-a
>>>>     mov r3,#1
>>>>
>>>> .arch armv4t
>>>> .eabi_attribute 6,2
>>>>     bx lr
>>>>
>>>> $ arm-none-linux-gnueabihf-as -mthumb bug.s
>>>> bug.s: Assembler messages:
>>>> bug.s:4: Error: invalid constant (1) after fixup
>>>>
>>>> previously this worked, but i'm not sure if the mix
>>>> of .arch directives and .syntax unified is valid here.
>>
>> It looks to me the failure is caused by the second ".arch armv4t" overrides the
>> first ".arch armv7-a", therefore the arch feature is lowerd to armv4t in
>> assembler fixup stage.  While "mov r3, #1" under thumb mode is only allowed
>> with ARMv6T2.
>>
>> This patch tightend the check from
>>
>>       if (fixP->fx_r_type == BFD_RELOC_ARM_T32_IMMEDIATE
>>           || fixP->fx_r_type == BFD_RELOC_ARM_T32_ADD_IMM)
>>
>> into:
>>
>>       if ((fixP->fx_r_type == BFD_RELOC_ARM_T32_IMMEDIATE
>>            && ARM_CPU_HAS_FEATURE (cpu_variant, arm_ext_v6t2))
>>           || fixP->fx_r_type == BFD_RELOC_ARM_T32_ADD_IMM)
>>
>>
>> thus the sequences in bug.s is not allowed.
> 
> Does .object_arch works for you? It seems you want the final object arch adjusted.
> 
> https://sourceware.org/ml/binutils/2006-09/msg00054.html

musl got fixed to use .object_arch:

http://git.musl-libc.org/cgit/musl/commit/?id=b261a24256792177a5f0531dbb25cc6267220ca5

thanks.

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2017-02-17 14:55 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-01 12:21 [ARM] Allow MOV/MOV.W to accept all possible immediates Jiong Wang
2016-11-03 11:37 ` Nick Clifton
2017-01-05 10:45 ` Szabolcs Nagy
2017-01-05 10:49   ` Szabolcs Nagy
2017-01-05 13:59     ` Jiong Wang
2017-01-05 14:16       ` Jiong Wang
2017-02-17 14:55         ` Szabolcs Nagy
2017-01-05 14:22       ` Jan Beulich

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).