Patchwork [arm-embedded] Request to backport thumb1 far jump patch to embedded 4.8 branch

login
register
mail settings
Submitter Terry Guo
Date Aug. 6, 2013, 6:17 a.m.
Message ID <000501ce926c$9f712a00$de537e00$@arm.com>
Download mbox | patch
Permalink /patch/264877/
State New
Headers show

Comments

Terry Guo - Aug. 6, 2013, 6:17 a.m.
Hello Joey,

The thumb1 far jump patch is about an optimization to avoid unnecessary lr
save instruction. It is now in trunk. Is it OK to back port it to embedded
4.8 branch?

BR,
Terry

gcc/ChangeLog.arm

 2013-08-05  Terry Guo  <terry.guo@arm.com>
 
	Backport from mainline r197956
	2013-04-15  Joey Ye  <joey.ye@arm.com>

	* config/arm/arm.c (thumb1_final_prescan_insn): Assert lr save
	for real far jump.
	(thumb_far_jump_used_p): Count instruction size and set
	far_jump_used.

gcc/testsuite/ChangeLog.arm

2013-08-05  Terry Guo  <terry.guo@arm.com>

	Backport from mainline r197956
	2013-04-15  Joey Ye  <joey.ye@arm.com>

	* gcc.target/arm/thumb1-far-jump-1.c: New test.
	* gcc.target/arm/thumb1-far-jump-2.c: New test.
Joey Ye - Aug. 6, 2013, 9:26 a.m.
OK for embedded 4.8 branch

- Joey

> -----Original Message-----
> From: Terry Guo
> Sent: Tuesday, August 06, 2013 14:17
> To: Joey Ye
> Cc: gcc-patches@gcc.gnu.org
> Subject: [arm-embedded] Request to backport thumb1 far jump patch to
> embedded 4.8 branch
> 
> Hello Joey,
> 
> The thumb1 far jump patch is about an optimization to avoid unnecessary lr
> save instruction. It is now in trunk. Is it OK to back port it to embedded
> 4.8 branch?
> 
> BR,
> Terry
> 
> gcc/ChangeLog.arm
> 
>  2013-08-05  Terry Guo  <terry.guo@arm.com>
> 
> 	Backport from mainline r197956
> 	2013-04-15  Joey Ye  <joey.ye@arm.com>
> 
> 	* config/arm/arm.c (thumb1_final_prescan_insn): Assert lr save
> 	for real far jump.
> 	(thumb_far_jump_used_p): Count instruction size and set
> 	far_jump_used.
> 
> gcc/testsuite/ChangeLog.arm
> 
> 2013-08-05  Terry Guo  <terry.guo@arm.com>
> 
> 	Backport from mainline r197956
> 	2013-04-15  Joey Ye  <joey.ye@arm.com>
> 
> 	* gcc.target/arm/thumb1-far-jump-1.c: New test.
> 	* gcc.target/arm/thumb1-far-jump-2.c: New test.

Patch

Index: gcc/ChangeLog.arm
===================================================================
--- gcc/ChangeLog.arm	(revision 201517)
+++ gcc/ChangeLog.arm	(working copy)
@@ -1,5 +1,15 @@ 
 2013-08-05  Terry Guo  <terry.guo@arm.com>
 
+	Backport from mainline r197956
+	2013-04-15  Joey Ye  <joey.ye@arm.com>
+
+	* config/arm/arm.c (thumb1_final_prescan_insn): Assert lr save
+	for real far jump.
+	(thumb_far_jump_used_p): Count instruction size and set
+	far_jump_used.
+
+2013-08-05  Terry Guo  <terry.guo@arm.com>
+
 	Backport from mainline r197153
 	2013-03-27  Terry Guo  <terry.guo@arm.com>
 
Index: gcc/testsuite/gcc.target/arm/thumb1-far-jump-1.c
===================================================================
--- gcc/testsuite/gcc.target/arm/thumb1-far-jump-1.c	(revision 0)
+++ gcc/testsuite/gcc.target/arm/thumb1-far-jump-1.c	(working copy)
@@ -0,0 +1,34 @@ 
+/* Check for thumb1 far jump. Shouldn't save lr for small leaf functions
+ * even with a branch in it.  */
+/* { dg-options "-Os" } */
+/* { dg-skip-if "" { ! { arm_thumb1 } } } */
+
+void f()
+{
+  for (;;);
+}
+
+volatile int g;
+void f2(int i)
+{
+  if (i) g=0;
+}
+
+void f3(int i)
+{
+  if (i) {
+    g=0;
+    g=1;
+    g=2;
+    g=3;
+    g=4;
+    g=5;
+    g=6;
+    g=7;
+    g=8;
+    g=9;
+  }
+}
+
+/* { dg-final { scan-assembler-not "push.*lr" } } */
+
Index: gcc/testsuite/gcc.target/arm/thumb1-far-jump-2.c
===================================================================
--- gcc/testsuite/gcc.target/arm/thumb1-far-jump-2.c	(revision 0)
+++ gcc/testsuite/gcc.target/arm/thumb1-far-jump-2.c	(working copy)
@@ -0,0 +1,57 @@ 
+/* Check for thumb1 far jump. This is the extreme case that far jump
+ * will be used with minimum number of instructions. By passing this case
+ * it means the heuristic of saving lr for far jump meets the most extreme
+ * requirement.  */
+/* { dg-options "-Os" } */
+/* { dg-skip-if "" { ! { arm_thumb1 } } } */
+
+volatile register r4 asm("r4");
+void f3(int i)
+{
+#define GO(n) \
+  extern volatile int g_##n; \
+  r4=(int)&g_##n;
+
+#define GO8(n) \
+  GO(n##_0) \
+  GO(n##_1) \
+  GO(n##_2) \
+  GO(n##_3) \
+  GO(n##_4) \
+  GO(n##_5) \
+  GO(n##_6) \
+  GO(n##_7)
+
+#define GO64(n) \
+  GO8(n##_0) \
+  GO8(n##_1) \
+  GO8(n##_2) \
+  GO8(n##_3) \
+  GO8(n##_4) \
+  GO8(n##_5) \
+  GO8(n##_6) \
+  GO8(n##_7) \
+
+#define GO498(n) \
+  GO64(n##_0) \
+  GO64(n##_1) \
+  GO64(n##_2) \
+  GO64(n##_3) \
+  GO64(n##_4) \
+  GO64(n##_5) \
+  GO64(n##_6) \
+  GO8(n##_0) \
+  GO8(n##_1) \
+  GO8(n##_2) \
+  GO8(n##_3) \
+  GO8(n##_4) \
+  GO8(n##_5) \
+  GO(n##_0) \
+  GO(n##_1) \
+
+  if (i) {
+    GO498(0);
+  }
+}
+
+/* { dg-final { scan-assembler "push.*lr" } } */
Index: gcc/testsuite/ChangeLog.arm
===================================================================
--- gcc/testsuite/ChangeLog.arm	(revision 0)
+++ gcc/testsuite/ChangeLog.arm	(working copy)
@@ -0,0 +1,7 @@ 
+2013-08-05  Terry Guo  <terry.guo@arm.com>
+
+	Backport from mainline r197956
+	2013-04-15  Joey Ye  <joey.ye@arm.com>
+
+	* gcc.target/arm/thumb1-far-jump-1.c: New test.
+	* gcc.target/arm/thumb1-far-jump-2.c: New test.
Index: gcc/config/arm/arm.c
===================================================================
--- gcc/config/arm/arm.c	(revision 201517)
+++ gcc/config/arm/arm.c	(working copy)
@@ -22577,6 +22577,11 @@ 
       else if (conds != CONDS_NOCOND)
 	cfun->machine->thumb1_cc_insn = NULL_RTX;
     }
+
+    /* Check if unexpected far jump is used.  */
+    if (cfun->machine->lr_save_eliminated
+        && get_attr_far_jump (insn) == FAR_JUMP_YES)
+      internal_error("Unexpected thumb1 far jump");
 }
 
 int
@@ -22602,6 +22607,8 @@ 
 thumb_far_jump_used_p (void)
 {
   rtx insn;
+  bool far_jump = false;
+  unsigned int func_size = 0;
 
   /* This test is only important for leaf functions.  */
   /* assert (!leaf_function_p ()); */
@@ -22657,6 +22664,26 @@ 
 	  && get_attr_far_jump (insn) == FAR_JUMP_YES
 	  )
 	{
+	  far_jump = true;
+	}
+      func_size += get_attr_length (insn);
+    }
+
+  /* Attribute far_jump will always be true for thumb1 before
+     shorten_branch pass.  So checking far_jump attribute before
+     shorten_branch isn't much useful.
+
+     Following heuristic tries to estimate more accruately if a far jump
+     may finally be used.  The heuristic is very conservative as there is
+     no chance to roll-back the decision of not to use far jump.
+
+     Thumb1 long branch offset is -2048 to 2046.  The worst case is each
+     2-byte insn is assiociated with a 4 byte constant pool.  Using
+     function size 2048/3 as the threshold is conservative enough.  */
+  if (far_jump)
+    {
+      if ((func_size * 3) >= 2048)
+        {
 	  /* Record the fact that we have decided that
 	     the function does use far jumps.  */
 	  cfun->machine->far_jump_used = 1;