This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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
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

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]