[PATCH, GAS/ARM] Fix expansion of ldr pseudo instruction

[PATCH, GAS/ARM] Fix expansion of ldr pseudo instruction

[Thomas Preudhomme]

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

Hi,

The LDR rX, =cst pseudo-instruction suffers from two issues for loading
integer constants in Thumb mode:

- movs is used if the constant and register can be encoded using that
   instruction which leads to unexpected behavior due to its flag-setting
   behavior
- mov.w, movw and mvn are used for r13 (sp) and r15 (pc) but these
   encoding are marked as UNPREDICTABLE

This patch fixes those issues and update testing accordingly.

ChangeLog entry is as follows:

*** gas/ChangeLog ***

2017-04-20  Thomas Preud'homme  <thomas.preudhomme@arm.com>

	* config/tc-arm.c (move_or_literal_pool): Remove code generating MOVS.
	Forbid MOV.W and MOVW if destination is SP or PC.
	* testsuite/gas/arm/thumb2_ldr_immediate_highregs_armv6t2.s: Explain
	expectation of LDR not generating a MOVS for low registers and small
	constants.  Add tests of MOVW generation.
	* testsuite/gas/arm/thumb2_ldr_immediate_highregs_armv6t2.d: Update
	expected disassembly.


Testsuite shows no regression with this patch.

Is this ok for master?

Best regards,

Thomas

[-- Attachment #2: fix_ldr_pseudo_insn_expansion.patch --]
[-- Type: text/x-patch, Size: 4912 bytes --]

diff --git a/gas/config/tc-arm.c b/gas/config/tc-arm.c
index 8098224d9568aedd91e0e8e1f19628458ca3246b..65a67d3848f84149d86310fbff6730f07d79ad21 100644
--- a/gas/config/tc-arm.c
+++ b/gas/config/tc-arm.c
@@ -7955,17 +7955,13 @@ move_or_literal_pool (int i, enum lit_type t, bfd_boolean mode_3)
 	{
 	  if (thumb_p)
 	    {
-	      /* This can be encoded only for a low register.  */
-	      if ((v & ~0xFF) == 0 && (inst.operands[i].reg < 8))
-		{
-		  /* This can be done with a mov(1) instruction.  */
-		  inst.instruction = T_OPCODE_MOV_I8 | (inst.operands[i].reg << 8);
-		  inst.instruction |= v;
-		  return TRUE;
-		}
+	      /* LDR should not use lead in a flag-setting instruction being
+		 chosen so we do not check whether movs can be used.  */
 
-	      if (ARM_CPU_HAS_FEATURE (cpu_variant, arm_ext_v6t2)
+	      if ((ARM_CPU_HAS_FEATURE (cpu_variant, arm_ext_v6t2)
 		  || ARM_CPU_HAS_FEATURE (cpu_variant, arm_ext_v6t2_v8m))
+		  && inst.operands[i].reg != 13
+		  && inst.operands[i].reg != 15)
 		{
 		  /* Check if on thumb2 it can be done with a mov.w, mvn or
 		     movw instruction.  */
diff --git a/gas/testsuite/gas/arm/thumb2_ldr_immediate_highregs_armv6t2.d b/gas/testsuite/gas/arm/thumb2_ldr_immediate_highregs_armv6t2.d
index 55b5f17887f94e894f5d02a4fcbb9c0d10d1227b..7afc1350e536798c48fabacb62adc079d5b13add 100644
--- a/gas/testsuite/gas/arm/thumb2_ldr_immediate_highregs_armv6t2.d
+++ b/gas/testsuite/gas/arm/thumb2_ldr_immediate_highregs_armv6t2.d
@@ -6,19 +6,23 @@
 .*: +file format .*arm.*
 
 Disassembly of section \.text:
-0[0-9a-f]+ <[^>]+> 2000[[:space:]]+movs[[:space:]]+r0, #0.*
-0[0-9a-f]+ <[^>]+> 2108[[:space:]]+movs[[:space:]]+r1, #8.*
-0[0-9a-f]+ <[^>]+> 2251[[:space:]]+movs[[:space:]]+r2, #81.*
-0[0-9a-f]+ <[^>]+> 231f[[:space:]]+movs[[:space:]]+r3, #31.*
-0[0-9a-f]+ <[^>]+> 242f[[:space:]]+movs[[:space:]]+r4, #47.*
-0[0-9a-f]+ <[^>]+> 253f[[:space:]]+movs[[:space:]]+r5, #63.*
-0[0-9a-f]+ <[^>]+> 2680[[:space:]]+movs[[:space:]]+r6, #128.*
-0[0-9a-f]+ <[^>]+> 27ff[[:space:]]+movs[[:space:]]+r7, #255.*
+0[0-9a-f]+ <[^>]+> f04f 0000[[:space:]]+mov\.w[[:space:]]+r0, #0.*
+0[0-9a-f]+ <[^>]+> f04f 0108[[:space:]]+mov\.w[[:space:]]+r1, #8.*
+0[0-9a-f]+ <[^>]+> f04f 0251[[:space:]]+mov\.w[[:space:]]+r2, #81.*
+0[0-9a-f]+ <[^>]+> f04f 031f[[:space:]]+mov\.w[[:space:]]+r3, #31.*
+0[0-9a-f]+ <[^>]+> f04f 042f[[:space:]]+mov\.w[[:space:]]+r4, #47.*
+0[0-9a-f]+ <[^>]+> f04f 053f[[:space:]]+mov\.w[[:space:]]+r5, #63.*
+0[0-9a-f]+ <[^>]+> f04f 0680[[:space:]]+mov\.w[[:space:]]+r6, #128.*
+0[0-9a-f]+ <[^>]+> f04f 07ff[[:space:]]+mov\.w[[:space:]]+r7, #255.*
 0[0-9a-f]+ <[^>]+> f04f 0800[[:space:]]+mov\.w[[:space:]]+r8, #0.*
 0[0-9a-f]+ <[^>]+> f04f 0908[[:space:]]+mov\.w[[:space:]]+r9, #8.*
 0[0-9a-f]+ <[^>]+> f04f 0a51[[:space:]]+mov\.w[[:space:]]+sl, #81.*
 0[0-9a-f]+ <[^>]+> f04f 0b1f[[:space:]]+mov\.w[[:space:]]+fp, #31.*
 0[0-9a-f]+ <[^>]+> f04f 0c2f[[:space:]]+mov\.w[[:space:]]+ip, #47.*
-0[0-9a-f]+ <[^>]+> f04f 0d3f[[:space:]]+mov\.w[[:space:]]+sp, #63.*
 0[0-9a-f]+ <[^>]+> f04f 0e80[[:space:]]+mov\.w[[:space:]]+lr, #128.*
-0[0-9a-f]+ <[^>]+> f04f 0fff[[:space:]]+mov\.w[[:space:]]+pc, #255.*
+0[0-9a-f]+ <[^>]+> f64f 78ff[[:space:]]+movw[[:space:]]+r8, #65535.*
+0[0-9a-f]+ <[^>]+> f24f 09f0[[:space:]]+movw[[:space:]]+r9, #61680.*
+0[0-9a-f]+ <[^>]+> f8df d004[[:space:]]+ldr\.w[[:space:]]+sp, \[pc, #4\].*
+0[0-9a-f]+ <[^>]+> f8df f004[[:space:]]+ldr\.w[[:space:]]+pc, \[pc, #4\].*
+0[0-9a-f]+ <[^>]+> 0000003f[[:space:]]+.word[[:space:]]+0x0000003f.*
+0[0-9a-f]+ <[^>]+> 000000ff[[:space:]]+.word[[:space:]]+0x000000ff.*
diff --git a/gas/testsuite/gas/arm/thumb2_ldr_immediate_highregs_armv6t2.s b/gas/testsuite/gas/arm/thumb2_ldr_immediate_highregs_armv6t2.s
index d2254101860f0fbc026dccb90076499cc2542b1a..b473857223603c946e4b5c923f4f9dfd110d5f6c 100644
--- a/gas/testsuite/gas/arm/thumb2_ldr_immediate_highregs_armv6t2.s
+++ b/gas/testsuite/gas/arm/thumb2_ldr_immediate_highregs_armv6t2.s
@@ -2,8 +2,8 @@
 	.syntax unified
 	.thumb_func
 thumb2_ldr:
-	# These can be encoded into movs since constant is small
-	# And register can be encoded in 3 bits
+	# These must be encoded into mov.w despite constant and register being
+	# small enough as ldr should not generate a flag-setting instruction.
 	ldr r0,=0x00
 	ldr r1,=0x08
 	ldr r2,=0x51
@@ -12,13 +12,19 @@ thumb2_ldr:
 	ldr r5,=0x3F
 	ldr r6,=0x80
 	ldr r7,=0xFF
-	# These shall be encoded into mov.w
-	# Since register cannot be encoded in 3 bits
+	# These shall be encoded into mov.w since register cannot be encoded in
+	# 3 bits
 	ldr r8,=0x00
 	ldr r9,=0x08
 	ldr r10,=0x51
 	ldr r11,=0x1F
 	ldr r12,=0x2F
-	ldr r13,=0x3F
 	ldr r14,=0x80
+	# These shall be encoded into movw since immediate cannot be encoded
+	# with mov.w
+	ldr r8,=0xFFFF
+	ldr r9,=0xF0F0
+	# These should be encoded as ldr since mov immediate is unpredictable
+	# for sp and pc
+	ldr r13,=0x3F
 	ldr r15,=0xFF

Re: [PATCH, GAS/ARM] Fix expansion of ldr pseudo instruction

[Nick Clifton]

Hi Thomas,

> 2017-04-20  Thomas Preud'homme  <thomas.preudhomme@arm.com>
> 
>      * config/tc-arm.c (move_or_literal_pool): Remove code generating MOVS.
>      Forbid MOV.W and MOVW if destination is SP or PC.
>      * testsuite/gas/arm/thumb2_ldr_immediate_highregs_armv6t2.s: Explain
>      expectation of LDR not generating a MOVS for low registers and small
>      constants.  Add tests of MOVW generation.
>      * testsuite/gas/arm/thumb2_ldr_immediate_highregs_armv6t2.d: Update
>      expected disassembly.

Approved - please apply.

Cheers
  Nick

Re: [PATCH, GAS/ARM] Fix expansion of ldr pseudo instruction

[Thomas Preudhomme]

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

Patch applies cleanly on binutils-2_28-branch and shows no testsuite regression. 
Is this ok to commit to binutils 2.28?

Best regards,

Thomas

On 24/04/17 14:07, Nick Clifton wrote:
> Hi Thomas,
>
>> 2017-04-20  Thomas Preud'homme  <thomas.preudhomme@arm.com>
>>
>>      * config/tc-arm.c (move_or_literal_pool): Remove code generating MOVS.
>>      Forbid MOV.W and MOVW if destination is SP or PC.
>>      * testsuite/gas/arm/thumb2_ldr_immediate_highregs_armv6t2.s: Explain
>>      expectation of LDR not generating a MOVS for low registers and small
>>      constants.  Add tests of MOVW generation.
>>      * testsuite/gas/arm/thumb2_ldr_immediate_highregs_armv6t2.d: Update
>>      expected disassembly.
>
> Approved - please apply.
>
> Cheers
>   Nick
>

[-- Attachment #2: fix_ldr_movs_expansion.patch --]
[-- Type: text/x-patch, Size: 5700 bytes --]

diff --git a/gas/ChangeLog b/gas/ChangeLog
index d6ab8a1e344f0af622d02a7212805d867ed06e4a..de32b8eb2a9ae994ba3e7844f80cb2c047ede1e6 100644
--- a/gas/ChangeLog
+++ b/gas/ChangeLog
@@ -1,3 +1,13 @@
+2017-04-24  Thomas Preud'homme  <thomas.preudhomme@arm.com>
+
+	* config/tc-arm.c (move_or_literal_pool): Remove code generating MOVS.
+	Forbid MOV.W and MOVW if destination is SP or PC.
+	* testsuite/gas/arm/thumb2_ldr_immediate_highregs_armv6t2.s: Explain
+	expectation of LDR not generating a MOVS for low registers and small
+	constants.  Add tests of MOVW generation.
+	* testsuite/gas/arm/thumb2_ldr_immediate_highregs_armv6t2.d: Update
+	expected disassembly.
+
 2017-04-03  Palmer Dabbelt  <palmer@dabbelt.com>
 
 	* config/tc-riscv.c (riscv_clear_subsets): Cast argument to free to
diff --git a/gas/config/tc-arm.c b/gas/config/tc-arm.c
index 60bda5107079f97ad963df64a3d4bc1bbf36b364..0820b96891f484f554d91b9e7f17edabc2a106c6 100644
--- a/gas/config/tc-arm.c
+++ b/gas/config/tc-arm.c
@@ -7955,17 +7955,13 @@ move_or_literal_pool (int i, enum lit_type t, bfd_boolean mode_3)
 	{
 	  if (thumb_p)
 	    {
-	      /* This can be encoded only for a low register.  */
-	      if ((v & ~0xFF) == 0 && (inst.operands[i].reg < 8))
-		{
-		  /* This can be done with a mov(1) instruction.  */
-		  inst.instruction = T_OPCODE_MOV_I8 | (inst.operands[i].reg << 8);
-		  inst.instruction |= v;
-		  return TRUE;
-		}
+	      /* LDR should not use lead in a flag-setting instruction being
+		 chosen so we do not check whether movs can be used.  */
 
-	      if (ARM_CPU_HAS_FEATURE (cpu_variant, arm_ext_v6t2)
+	      if ((ARM_CPU_HAS_FEATURE (cpu_variant, arm_ext_v6t2)
 		  || ARM_CPU_HAS_FEATURE (cpu_variant, arm_ext_v6t2_v8m))
+		  && inst.operands[i].reg != 13
+		  && inst.operands[i].reg != 15)
 		{
 		  /* Check if on thumb2 it can be done with a mov.w, mvn or
 		     movw instruction.  */
diff --git a/gas/testsuite/gas/arm/thumb2_ldr_immediate_highregs_armv6t2.d b/gas/testsuite/gas/arm/thumb2_ldr_immediate_highregs_armv6t2.d
index 55b5f17887f94e894f5d02a4fcbb9c0d10d1227b..7afc1350e536798c48fabacb62adc079d5b13add 100644
--- a/gas/testsuite/gas/arm/thumb2_ldr_immediate_highregs_armv6t2.d
+++ b/gas/testsuite/gas/arm/thumb2_ldr_immediate_highregs_armv6t2.d
@@ -6,19 +6,23 @@
 .*: +file format .*arm.*
 
 Disassembly of section \.text:
-0[0-9a-f]+ <[^>]+> 2000[[:space:]]+movs[[:space:]]+r0, #0.*
-0[0-9a-f]+ <[^>]+> 2108[[:space:]]+movs[[:space:]]+r1, #8.*
-0[0-9a-f]+ <[^>]+> 2251[[:space:]]+movs[[:space:]]+r2, #81.*
-0[0-9a-f]+ <[^>]+> 231f[[:space:]]+movs[[:space:]]+r3, #31.*
-0[0-9a-f]+ <[^>]+> 242f[[:space:]]+movs[[:space:]]+r4, #47.*
-0[0-9a-f]+ <[^>]+> 253f[[:space:]]+movs[[:space:]]+r5, #63.*
-0[0-9a-f]+ <[^>]+> 2680[[:space:]]+movs[[:space:]]+r6, #128.*
-0[0-9a-f]+ <[^>]+> 27ff[[:space:]]+movs[[:space:]]+r7, #255.*
+0[0-9a-f]+ <[^>]+> f04f 0000[[:space:]]+mov\.w[[:space:]]+r0, #0.*
+0[0-9a-f]+ <[^>]+> f04f 0108[[:space:]]+mov\.w[[:space:]]+r1, #8.*
+0[0-9a-f]+ <[^>]+> f04f 0251[[:space:]]+mov\.w[[:space:]]+r2, #81.*
+0[0-9a-f]+ <[^>]+> f04f 031f[[:space:]]+mov\.w[[:space:]]+r3, #31.*
+0[0-9a-f]+ <[^>]+> f04f 042f[[:space:]]+mov\.w[[:space:]]+r4, #47.*
+0[0-9a-f]+ <[^>]+> f04f 053f[[:space:]]+mov\.w[[:space:]]+r5, #63.*
+0[0-9a-f]+ <[^>]+> f04f 0680[[:space:]]+mov\.w[[:space:]]+r6, #128.*
+0[0-9a-f]+ <[^>]+> f04f 07ff[[:space:]]+mov\.w[[:space:]]+r7, #255.*
 0[0-9a-f]+ <[^>]+> f04f 0800[[:space:]]+mov\.w[[:space:]]+r8, #0.*
 0[0-9a-f]+ <[^>]+> f04f 0908[[:space:]]+mov\.w[[:space:]]+r9, #8.*
 0[0-9a-f]+ <[^>]+> f04f 0a51[[:space:]]+mov\.w[[:space:]]+sl, #81.*
 0[0-9a-f]+ <[^>]+> f04f 0b1f[[:space:]]+mov\.w[[:space:]]+fp, #31.*
 0[0-9a-f]+ <[^>]+> f04f 0c2f[[:space:]]+mov\.w[[:space:]]+ip, #47.*
-0[0-9a-f]+ <[^>]+> f04f 0d3f[[:space:]]+mov\.w[[:space:]]+sp, #63.*
 0[0-9a-f]+ <[^>]+> f04f 0e80[[:space:]]+mov\.w[[:space:]]+lr, #128.*
-0[0-9a-f]+ <[^>]+> f04f 0fff[[:space:]]+mov\.w[[:space:]]+pc, #255.*
+0[0-9a-f]+ <[^>]+> f64f 78ff[[:space:]]+movw[[:space:]]+r8, #65535.*
+0[0-9a-f]+ <[^>]+> f24f 09f0[[:space:]]+movw[[:space:]]+r9, #61680.*
+0[0-9a-f]+ <[^>]+> f8df d004[[:space:]]+ldr\.w[[:space:]]+sp, \[pc, #4\].*
+0[0-9a-f]+ <[^>]+> f8df f004[[:space:]]+ldr\.w[[:space:]]+pc, \[pc, #4\].*
+0[0-9a-f]+ <[^>]+> 0000003f[[:space:]]+.word[[:space:]]+0x0000003f.*
+0[0-9a-f]+ <[^>]+> 000000ff[[:space:]]+.word[[:space:]]+0x000000ff.*
diff --git a/gas/testsuite/gas/arm/thumb2_ldr_immediate_highregs_armv6t2.s b/gas/testsuite/gas/arm/thumb2_ldr_immediate_highregs_armv6t2.s
index d2254101860f0fbc026dccb90076499cc2542b1a..b473857223603c946e4b5c923f4f9dfd110d5f6c 100644
--- a/gas/testsuite/gas/arm/thumb2_ldr_immediate_highregs_armv6t2.s
+++ b/gas/testsuite/gas/arm/thumb2_ldr_immediate_highregs_armv6t2.s
@@ -2,8 +2,8 @@
 	.syntax unified
 	.thumb_func
 thumb2_ldr:
-	# These can be encoded into movs since constant is small
-	# And register can be encoded in 3 bits
+	# These must be encoded into mov.w despite constant and register being
+	# small enough as ldr should not generate a flag-setting instruction.
 	ldr r0,=0x00
 	ldr r1,=0x08
 	ldr r2,=0x51
@@ -12,13 +12,19 @@ thumb2_ldr:
 	ldr r5,=0x3F
 	ldr r6,=0x80
 	ldr r7,=0xFF
-	# These shall be encoded into mov.w
-	# Since register cannot be encoded in 3 bits
+	# These shall be encoded into mov.w since register cannot be encoded in
+	# 3 bits
 	ldr r8,=0x00
 	ldr r9,=0x08
 	ldr r10,=0x51
 	ldr r11,=0x1F
 	ldr r12,=0x2F
-	ldr r13,=0x3F
 	ldr r14,=0x80
+	# These shall be encoded into movw since immediate cannot be encoded
+	# with mov.w
+	ldr r8,=0xFFFF
+	ldr r9,=0xF0F0
+	# These should be encoded as ldr since mov immediate is unpredictable
+	# for sp and pc
+	ldr r13,=0x3F
 	ldr r15,=0xFF

Re: [PATCH, GAS/ARM] Fix expansion of ldr pseudo instruction

[Ramana Radhakrishnan]

On Wed, Jun 14, 2017 at 9:49 AM, Thomas Preudhomme
<thomas.preudhomme@foss.arm.com> wrote:
> Patch applies cleanly on binutils-2_28-branch and shows no testsuite
> regression. Is this ok to commit to binutils 2.28?
>

can you mark this as fixing PR21590 in your changelog for 2.28 ?

However you need an ACK from Tristan about 2.28.

Also could you please note the sha1 which fixed this on trunk on the
bz for posterity ?

regards
Ramana


> Best regards,
>
> Thomas
>
>
> On 24/04/17 14:07, Nick Clifton wrote:
>>
>> Hi Thomas,
>>
>>> 2017-04-20  Thomas Preud'homme  <thomas.preudhomme@arm.com>
>>>
>>>      * config/tc-arm.c (move_or_literal_pool): Remove code generating
>>> MOVS.
>>>      Forbid MOV.W and MOVW if destination is SP or PC.
>>>      * testsuite/gas/arm/thumb2_ldr_immediate_highregs_armv6t2.s: Explain
>>>      expectation of LDR not generating a MOVS for low registers and small
>>>      constants.  Add tests of MOVW generation.
>>>      * testsuite/gas/arm/thumb2_ldr_immediate_highregs_armv6t2.d: Update
>>>      expected disassembly.
>>
>>
>> Approved - please apply.
>>
>> Cheers
>>   Nick
>>
>

Re: [PATCH, GAS/ARM] Fix expansion of ldr pseudo instruction

[Thomas Preudhomme]

On 14/06/17 10:11, Ramana Radhakrishnan wrote:
> On Wed, Jun 14, 2017 at 9:49 AM, Thomas Preudhomme
> <thomas.preudhomme@foss.arm.com> wrote:
>> Patch applies cleanly on binutils-2_28-branch and shows no testsuite
>> regression. Is this ok to commit to binutils 2.28?
>>
>
> can you mark this as fixing PR21590 in your changelog for 2.28 ?

Sure, please ignore the ChangeLog in the patch, this is coming from the 
cherry-pick. The proposed ChangeLog would be:

2017-06-14  Thomas Preud'homme  <thomas.preudhomme@arm.com>

	Backport from mainline
	2017-04-24  Thomas Preud'homme  <thomas.preudhomme@arm.com>

	PR 21590
	* config/tc-arm.c (move_or_literal_pool): Remove code generating MOVS.
	Forbid MOV.W and MOVW if destination is SP or PC.
	* testsuite/gas/arm/thumb2_ldr_immediate_highregs_armv6t2.s: Explain
	expectation of LDR not generating a MOVS for low registers and small
	constants.  Add tests of MOVW generation.
	* testsuite/gas/arm/thumb2_ldr_immediate_highregs_armv6t2.d: Update
	expected disassembly.

>
> However you need an ACK from Tristan about 2.28.

Yes indeed, my mistake.

>
> Also could you please note the sha1 which fixed this on trunk on the
> bz for posterity ?

Done.

Best regards,

Thomas

>
> regards
> Ramana
>
>
>> Best regards,
>>
>> Thomas
>>
>>
>> On 24/04/17 14:07, Nick Clifton wrote:
>>>
>>> Hi Thomas,
>>>
>>>> 2017-04-20  Thomas Preud'homme  <thomas.preudhomme@arm.com>
>>>>
>>>>      * config/tc-arm.c (move_or_literal_pool): Remove code generating
>>>> MOVS.
>>>>      Forbid MOV.W and MOVW if destination is SP or PC.
>>>>      * testsuite/gas/arm/thumb2_ldr_immediate_highregs_armv6t2.s: Explain
>>>>      expectation of LDR not generating a MOVS for low registers and small
>>>>      constants.  Add tests of MOVW generation.
>>>>      * testsuite/gas/arm/thumb2_ldr_immediate_highregs_armv6t2.d: Update
>>>>      expected disassembly.
>>>
>>>
>>> Approved - please apply.
>>>
>>> Cheers
>>>   Nick
>>>
>>

Re: [PATCH, GAS/ARM, ping] Fix expansion of ldr pseudo instruction

[Thomas Preudhomme]

Ping Tristan?

Best regards,

Thomas

On 14/06/17 10:25, Thomas Preudhomme wrote:
>
>
> On 14/06/17 10:11, Ramana Radhakrishnan wrote:
>> On Wed, Jun 14, 2017 at 9:49 AM, Thomas Preudhomme
>> <thomas.preudhomme@foss.arm.com> wrote:
>>> Patch applies cleanly on binutils-2_28-branch and shows no testsuite
>>> regression. Is this ok to commit to binutils 2.28?
>>>
>>
>> can you mark this as fixing PR21590 in your changelog for 2.28 ?
>
> Sure, please ignore the ChangeLog in the patch, this is coming from the
> cherry-pick. The proposed ChangeLog would be:
>
> 2017-06-14  Thomas Preud'homme  <thomas.preudhomme@arm.com>
>
>     Backport from mainline
>     2017-04-24  Thomas Preud'homme  <thomas.preudhomme@arm.com>
>
>     PR 21590
>     * config/tc-arm.c (move_or_literal_pool): Remove code generating MOVS.
>     Forbid MOV.W and MOVW if destination is SP or PC.
>     * testsuite/gas/arm/thumb2_ldr_immediate_highregs_armv6t2.s: Explain
>     expectation of LDR not generating a MOVS for low registers and small
>     constants.  Add tests of MOVW generation.
>     * testsuite/gas/arm/thumb2_ldr_immediate_highregs_armv6t2.d: Update
>     expected disassembly.
>
>>
>> However you need an ACK from Tristan about 2.28.
>
> Yes indeed, my mistake.
>
>>
>> Also could you please note the sha1 which fixed this on trunk on the
>> bz for posterity ?
>
> Done.
>
> Best regards,
>
> Thomas
>
>>
>> regards
>> Ramana
>>
>>
>>> Best regards,
>>>
>>> Thomas
>>>
>>>
>>> On 24/04/17 14:07, Nick Clifton wrote:
>>>>
>>>> Hi Thomas,
>>>>
>>>>> 2017-04-20  Thomas Preud'homme  <thomas.preudhomme@arm.com>
>>>>>
>>>>>      * config/tc-arm.c (move_or_literal_pool): Remove code generating
>>>>> MOVS.
>>>>>      Forbid MOV.W and MOVW if destination is SP or PC.
>>>>>      * testsuite/gas/arm/thumb2_ldr_immediate_highregs_armv6t2.s: Explain
>>>>>      expectation of LDR not generating a MOVS for low registers and small
>>>>>      constants.  Add tests of MOVW generation.
>>>>>      * testsuite/gas/arm/thumb2_ldr_immediate_highregs_armv6t2.d: Update
>>>>>      expected disassembly.
>>>>
>>>>
>>>> Approved - please apply.
>>>>
>>>> Cheers
>>>>   Nick
>>>>
>>>

Re: [PATCH, GAS/ARM, ping] Fix expansion of ldr pseudo instruction

[Tristan Gingold]

On 20/06/2017 11:03, Thomas Preudhomme wrote:
> Ping Tristan?

I've missed your mail, sorry.

Yes, that's ok.

Tristan.

>
> Best regards,
>
> Thomas
>
> On 14/06/17 10:25, Thomas Preudhomme wrote:
>>
>>
>> On 14/06/17 10:11, Ramana Radhakrishnan wrote:
>>> On Wed, Jun 14, 2017 at 9:49 AM, Thomas Preudhomme
>>> <thomas.preudhomme@foss.arm.com> wrote:
>>>> Patch applies cleanly on binutils-2_28-branch and shows no testsuite
>>>> regression. Is this ok to commit to binutils 2.28?
>>>>
>>>
>>> can you mark this as fixing PR21590 in your changelog for 2.28 ?
>>
>> Sure, please ignore the ChangeLog in the patch, this is coming from the
>> cherry-pick. The proposed ChangeLog would be:
>>
>> 2017-06-14  Thomas Preud'homme  <thomas.preudhomme@arm.com>
>>
>>     Backport from mainline
>>     2017-04-24  Thomas Preud'homme  <thomas.preudhomme@arm.com>
>>
>>     PR 21590
>>     * config/tc-arm.c (move_or_literal_pool): Remove code generating
>> MOVS.
>>     Forbid MOV.W and MOVW if destination is SP or PC.
>>     * testsuite/gas/arm/thumb2_ldr_immediate_highregs_armv6t2.s: Explain
>>     expectation of LDR not generating a MOVS for low registers and small
>>     constants.  Add tests of MOVW generation.
>>     * testsuite/gas/arm/thumb2_ldr_immediate_highregs_armv6t2.d: Update
>>     expected disassembly.
>>
>>>
>>> However you need an ACK from Tristan about 2.28.
>>
>> Yes indeed, my mistake.
>>
>>>
>>> Also could you please note the sha1 which fixed this on trunk on the
>>> bz for posterity ?
>>
>> Done.
>>
>> Best regards,
>>
>> Thomas
>>
>>>
>>> regards
>>> Ramana
>>>
>>>
>>>> Best regards,
>>>>
>>>> Thomas
>>>>
>>>>
>>>> On 24/04/17 14:07, Nick Clifton wrote:
>>>>>
>>>>> Hi Thomas,
>>>>>
>>>>>> 2017-04-20  Thomas Preud'homme  <thomas.preudhomme@arm.com>
>>>>>>
>>>>>>      * config/tc-arm.c (move_or_literal_pool): Remove code generating
>>>>>> MOVS.
>>>>>>      Forbid MOV.W and MOVW if destination is SP or PC.
>>>>>>      * testsuite/gas/arm/thumb2_ldr_immediate_highregs_armv6t2.s:
>>>>>> Explain
>>>>>>      expectation of LDR not generating a MOVS for low registers
>>>>>> and small
>>>>>>      constants.  Add tests of MOVW generation.
>>>>>>      * testsuite/gas/arm/thumb2_ldr_immediate_highregs_armv6t2.d:
>>>>>> Update
>>>>>>      expected disassembly.
>>>>>
>>>>>
>>>>> Approved - please apply.
>>>>>
>>>>> Cheers
>>>>>   Nick
>>>>>
>>>>