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, ARM 1/7] Consolidate Thumb-1/Thumb-2 ISA detection


Hi,

This patch is part of a patch series to add support for ARMv8-M[1] to binutils. This specific patch fixes the code responsible for the Thumb ISA detection. ARM backend of GAS contains the macro ARM_ARCH_THUMB2 and its associated variable arm_ext_t2 to determine the extensions that makes a Thumb code being recognized as Thumb-2. However, these are misused in a number of cases and the definition of ARM_ARCH_THUMB2 include some extension that don't contain any Thumb-2 instruction. This patch cleans all that which paves the way for correct Thumb ISA detection when ARMv8-M Baseline comes into the picture.

[1] For a quick overview of ARMv8-M please refer to the initial cover letter.

The reasoning for each existing use of arm_arch_t2 is as follows:

* move_or_literal_pool: check is for testing availability of specific instructions so use the right extension bit for them
* handle_it_state: likewise
* md_assemble: comment is explicit, only architecture with Thumb-2 can use VFP so arm_arch_t2 is correct here
* md_convert_frag: checking , given a Thumb-2 instruction is present, whether v6t2 extension bit should be set -> no need if any Thumb-2 extension bit is already there so arm_arch_t2 is correct here
* md_apply_fix: checking for availability of bl/blx with extended range which is a feature of ARMv6t2

The patch also adds functions wide_insn_ok () to centralize the logic of what instruction can be promoted from 16bit to 32bit encoding and whether Thumb-2 is needed for a given wide encoding. Although these are two separate decisions to make, the logic is the same when ARMv8-M is out of the picture.

ChangeLog entries are as follow:


*** gas/ChangeLog ***

2015-12-14  Thomas Preud'homme  <thomas.preudhomme@arm.com>

        * config/tc-arm.c (move_or_literal_pool): Check mov.w, mvm and movw
        availability against arm_ext_v6t2 instead of checking arm_arch_t2,
        fixing comments along the way.
        (handle_it_state): Check arm_ext_v6t2 instead of arm_arch_t2 to
        generate IT instruction.
        (non_v6t2_wide_only_insn): New function.
        (md_assemble): Use above new function to check for invalid wide
        instruction for CPU Thumb ISA and to determine what Thumb extension
        bit is necessary for that instruction.
        (md_apply_fix): Use arm_ext_v6t2 instead of arm_arch_t2 to decide if
        branch is out of range.


*** include/opcode/ChangeLog ***

2015-12-10  Thomas Preud'homme  <thomas.preudhomme@arm.com>

        * arm.h (ARM_ARCH_THUMB2): Add comment explaining its meaning and
        remove extension bit not including any Thumb-2 instruction.


diff --git a/gas/config/tc-arm.c b/gas/config/tc-arm.c
index 7289a13994b8dce9f3967e0f124c1d1f99894af3..fddb31a9c1a4f654e6006293915a41f1b7d6d7af 100644
--- a/gas/config/tc-arm.c
+++ b/gas/config/tc-arm.c
@@ -7848,10 +7848,10 @@ move_or_literal_pool (int i, enum lit_type t, bfd_boolean mode_3)
 		  return TRUE;
 		}
 
-	      if (ARM_CPU_HAS_FEATURE (cpu_variant, arm_arch_t2))
+	      if (ARM_CPU_HAS_FEATURE (cpu_variant, arm_ext_v6t2))
 		{
-		  /* Check if on thumb2 it can be done with a mov.w or mvn.w
-		     instruction.  */
+		  /* Check if on thumb2 it can be done with a mov.w, mvn or
+		     movw instruction.  */
 		  unsigned int newimm;
 		  bfd_boolean isNegated;
 
@@ -7865,19 +7865,22 @@ move_or_literal_pool (int i, enum lit_type t, bfd_boolean mode_3)
 			isNegated = TRUE;
 		    }
 
+		  /* The number can be loaded with a mov.w or mvn
+		     instruction.  */
 		  if (newimm != (unsigned int) FAIL)
 		    {
-		      inst.instruction = (0xf04f0000
+		      inst.instruction = (0xf04f0000  /*  MOV.W.  */
 					  | (inst.operands[i].reg << 8));
+		      /* Change to MOVN.  */
 		      inst.instruction |= (isNegated ? 0x200000 : 0);
 		      inst.instruction |= (newimm & 0x800) << 15;
 		      inst.instruction |= (newimm & 0x700) << 4;
 		      inst.instruction |= (newimm & 0x0ff);
 		      return TRUE;
 		    }
+		  /* The number can be loaded with a movw instruction.  */
 		  else if ((v & ~0xFFFF) == 0)
 		    {
-		      /* The number can be loaded with a mov.w instruction.  */
 		      int imm = v & 0xFFFF;
 
 		      inst.instruction = 0xf2400000;  /* MOVW.  */
@@ -17536,7 +17539,7 @@ handle_it_state (void)
 	  else
 	    {
 	      if ((implicit_it_mode & IMPLICIT_IT_MODE_THUMB)
-		  && ARM_CPU_HAS_FEATURE (cpu_variant, arm_arch_t2))
+		  && ARM_CPU_HAS_FEATURE (cpu_variant, arm_ext_v6t2))
 		{
 		  /* Automatically generate the IT instruction.  */
 		  new_automatic_it_block (inst.cond);
@@ -17768,6 +17771,22 @@ in_it_block (void)
   return now_it.state != OUTSIDE_IT_BLOCK;
 }
 
+/* Given an OPCODE that is valid in at least one architecture that is not a
+   superset of ARMv6t2, returns whether it only has wide encoding(s).  */
+
+static bfd_boolean
+non_v6t2_wide_only_insn (const struct asm_opcode *opcode)
+{
+  /* Thumb-1 wide instruction.  */
+  if (opcode->tencode == do_t_blx
+      || opcode->tencode == do_t_branch23
+      || ARM_CPU_HAS_FEATURE (*opcode->tvariant, arm_ext_msr)
+      || ARM_CPU_HAS_FEATURE (*opcode->tvariant, arm_ext_barrier))
+    return TRUE;
+
+  return FALSE;
+}
+
 void
 md_assemble (char *str)
 {
@@ -17827,24 +17846,24 @@ md_assemble (char *str)
 	  return;
 	}
 
-      if (!ARM_CPU_HAS_FEATURE (variant, arm_ext_v6t2))
+      /* Two things are addressed here:
+	 1) Implicit require narrow instructions on Thumb-1.
+	    This avoids relaxation accidentally introducing Thumb-2
+	    instructions.
+	 2) Reject wide instructions in non Thumb-2 cores.
+
+	 Only instructions with narrow and wide variants need to be handled
+	 but selecting all non wide-only instructions is easier.  */
+      if (!ARM_CPU_HAS_FEATURE (variant, arm_ext_v6t2)
+	  && !non_v6t2_wide_only_insn (opcode))
 	{
-	  if (opcode->tencode != do_t_blx && opcode->tencode != do_t_branch23
-	      && !(ARM_CPU_HAS_FEATURE(*opcode->tvariant, arm_ext_msr)
-		   || ARM_CPU_HAS_FEATURE(*opcode->tvariant, arm_ext_barrier)))
+	  if (inst.size_req == 0)
+	    inst.size_req = 2;
+	  else if (inst.size_req == 4)
 	    {
-	      /* Two things are addressed here.
-		 1) Implicit require narrow instructions on Thumb-1.
-		    This avoids relaxation accidentally introducing Thumb-2
-		     instructions.
-		 2) Reject wide instructions in non Thumb-2 cores.  */
-	      if (inst.size_req == 0)
-		inst.size_req = 2;
-	      else if (inst.size_req == 4)
-		{
-		  as_bad (_("selected processor does not support `%s' in Thumb-2 mode"), str);
-		  return;
-		}
+	      as_bad (_("selected processor does not support `%s' in Thumb-2 "
+			"mode"), str);
+	      return;
 	    }
 	}
 
@@ -17879,13 +17898,10 @@ md_assemble (char *str)
       ARM_MERGE_FEATURE_SETS (thumb_arch_used, thumb_arch_used,
 			      *opcode->tvariant);
       /* Many Thumb-2 instructions also have Thumb-1 variants, so explicitly
-	 set those bits when Thumb-2 32-bit instructions are seen.  ie.
-	 anything other than bl/blx and v6-M instructions.
-	 The impact of relaxable instructions will be considered later after we
-	 finish all relaxation.  */
-      if ((inst.size == 4 && (inst.instruction & 0xf800e800) != 0xf000e800)
-	  && !(ARM_CPU_HAS_FEATURE (*opcode->tvariant, arm_ext_msr)
-	       || ARM_CPU_HAS_FEATURE (*opcode->tvariant, arm_ext_barrier)))
+	 set those bits when Thumb-2 32-bit instructions are seen.  The impact
+	 of relaxable instructions will be considered later after we finish all
+	 relaxation.  */
+      if (inst.size == 4 && !non_v6t2_wide_only_insn (opcode))
 	ARM_MERGE_FEATURE_SETS (thumb_arch_used, thumb_arch_used,
 				arm_ext_v6t2);
 
@@ -22853,7 +22869,7 @@ md_apply_fix (fixS *	fixP,
 
       if ((value & ~0x3fffff) && ((value & ~0x3fffff) != ~0x3fffff))
 	{
-	  if (!(ARM_CPU_HAS_FEATURE (cpu_variant, arm_arch_t2)))
+	  if (!(ARM_CPU_HAS_FEATURE (cpu_variant, arm_ext_v6t2)))
 	    as_bad_where (fixP->fx_file, fixP->fx_line, BAD_RANGE);
 	  else if ((value & ~0x1ffffff)
 		   && ((value & ~0x1ffffff) != ~0x1ffffff))
diff --git a/include/opcode/arm.h b/include/opcode/arm.h
index 286016bbf2a1bc9b3ee58eff9ffae5ccdf602e23..abbe6a82206450aa525b36f3679379d538fe6e1e 100644
--- a/include/opcode/arm.h
+++ b/include/opcode/arm.h
@@ -261,10 +261,10 @@
 #define ARM_ANY		ARM_FEATURE (-1, -1, 0)	/* Any basic core.  */
 #define ARM_FEATURE_ALL	ARM_FEATURE (-1, -1, -1)/* All CPU and FPU features.  */
 #define FPU_ANY_HARD	ARM_FEATURE_COPROC (FPU_FPA | FPU_VFP_HARD | FPU_MAVERICK)
+/* Extensions containing some Thumb-2 instructions.  If any is present, Thumb
+   ISA is Thumb-2.  */
 #define ARM_ARCH_THUMB2 ARM_FEATURE_CORE_LOW (ARM_EXT_V6T2 | ARM_EXT_V7	\
-					      | ARM_EXT_V7A | ARM_EXT_V7R \
-					      | ARM_EXT_V7M | ARM_EXT_DIV \
-					      | ARM_EXT_V8)
+					      | ARM_EXT_DIV | ARM_EXT_V8)
 /* v7-a+sec.  */
 #define ARM_ARCH_V7A_SEC ARM_FEATURE_CORE_LOW (ARM_AEXT_V7A | ARM_EXT_SEC)
 /* v7-a+mp+sec.  */


Tests done:

* No regression under binutils testsuite
* Toolchain was built successfully with and without the ARMv8-M support patches[2] with the following multilib list: armv6-m,armv7-m,armv7e-m,cortex-m7,armv8-m.base,armv8-m.main. The code generation for crtbegin.o, crtend.o, crti.o, crtn.o, libgcc.a, libgcov.a, libc.a, libg.a, libgloss-linux.a, libm.a, libnosys.a, librdimon.a, librdpmon.a, libstdc++.a and libsupc++.a is unchanged for all targets supported before the patches.
* Thumb-1 (default arch and --with-mode=thumb) and Thumb-2 (--with-arch=armv7-a --with-mode=thumb) GCC bootstrap using binutils with this patch
* No GCC testsuite regression on fast model compared to ARMv6s-M (Baseline) or ARMv7-M (Mainline)

[2] including this one, the ld one and the GCC one

Is this ok for master branch?

Best regards,

Thomas


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