public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH][GCC][ARM] Arm generates out of range conditional branches in Thumb2 (PR91816)
@ 2019-10-11 14:57 Stam Markianos-Wright
  2019-10-13 15:37 ` Ramana Radhakrishnan
  0 siblings, 1 reply; 15+ messages in thread
From: Stam Markianos-Wright @ 2019-10-11 14:57 UTC (permalink / raw)
  To: gcc-patches; +Cc: nd, James Greenhalgh, Richard Earnshaw

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

Hi all,

This is a patch for an issue where the compiler was generating a 
conditional branch in Thumb2, which was too far for b{cond} to handle.

This was originally reported at binutils:
https://sourceware.org/bugzilla/show_bug.cgi?id=24991

And then raised for GCC:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91816


As can be seen here:

http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0489c/Cihfddaf.html

the range of a 32-bit Thumb B{cond} is +/-1MB.

This is now checked for in arm.md and an unconditional branch is 
generated if the jump would be greater than 1MB.

New test has been written that checks this for: beq (if (a)), bne (if 
(a==1))

Patch bootstrapped and regression tested on arm-none-linux-gnueabihf, 
however, on my native Aarch32 setup the test times out when run as part 
of a big "make check-gcc" regression, but not when run individually.

Patch also regression tested on arm-none-eabi, arm-none-linux-gnueabi 
with no issues.

Also, I don't have commit rights yet, so could someone commit it on my 
behalf?

Thanks,
Stam Markianos-Wright



gcc/ChangeLog:

2019-10-11  Stamatis Markianos-Wright <stam.markianos-wright@arm.com>

	* config/arm/arm.md: Update b<cond> for Thumb2 range checks.
	* config/arm/arm.c: New function arm_gen_far_branch.
  	* config/arm/arm-protos.h: New function arm_gen_far_branch
	prototype.

gcc/testsuite/ChangeLog:

2019-10-11  Stamatis Markianos-Wright <stam.markianos-wright@arm.com>

  	* testsuite/gcc.target/arm/pr91816.c: New test.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch.patch --]
[-- Type: text/x-patch; name="patch.patch", Size: 8352 bytes --]

diff --git a/gcc/config/arm/arm-protos.h b/gcc/config/arm/arm-protos.h
index f995974f9bb..1dce333d1c3 100644
--- a/gcc/config/arm/arm-protos.h
+++ b/gcc/config/arm/arm-protos.h
@@ -570,4 +570,7 @@ void arm_parse_option_features (sbitmap, const cpu_arch_option *,
 
 void arm_initialize_isa (sbitmap, const enum isa_feature *);
 
+const char * arm_gen_far_branch (rtx *, int,const char * , const char *);
+
+
 #endif /* ! GCC_ARM_PROTOS_H */
diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 39e1a1ef9a2..1a693d2ddca 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -32139,6 +32139,31 @@ arm_run_selftests (void)
 }
 } /* Namespace selftest.  */
 
+
+/* Generate code to enable conditional branches in functions over 1 MiB.  */
+const char *
+arm_gen_far_branch (rtx * operands, int pos_label, const char * dest,
+			const char * branch_format)
+{
+  rtx_code_label * tmp_label = gen_label_rtx ();
+  char label_buf[256];
+  char buffer[128];
+  ASM_GENERATE_INTERNAL_LABEL (label_buf, dest , \
+			CODE_LABEL_NUMBER (tmp_label));
+  const char *label_ptr = arm_strip_name_encoding (label_buf);
+  rtx dest_label = operands[pos_label];
+  operands[pos_label] = tmp_label;
+
+  snprintf (buffer, sizeof (buffer), "%s%s", branch_format , label_ptr);
+  output_asm_insn (buffer, operands);
+
+  snprintf (buffer, sizeof (buffer), "b\t%%l0%d\n%s:", pos_label, label_ptr);
+  operands[pos_label] = dest_label;
+  output_asm_insn (buffer, operands);
+  return "";
+}
+
+
 #undef TARGET_RUN_TARGET_SELFTESTS
 #define TARGET_RUN_TARGET_SELFTESTS selftest::arm_run_selftests
 #endif /* CHECKING_P */
diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md
index f861c72ccfc..634fd0a59da 100644
--- a/gcc/config/arm/arm.md
+++ b/gcc/config/arm/arm.md
@@ -6686,9 +6686,16 @@
 ;; And for backward branches we have 
 ;;   (neg_range - neg_base_offs + pc_offs) = (neg_range - (-2 or -4) + 4).
 ;;
+;; In 16-bit Thumb these ranges are:
 ;; For a 'b'       pos_range = 2046, neg_range = -2048 giving (-2040->2048).
 ;; For a 'b<cond>' pos_range = 254,  neg_range = -256  giving (-250 ->256).
 
+;; In 32-bit Thumb these ranges are:
+;; For a 'b'       +/- 16MB is not checked for.
+;; For a 'b<cond>' pos_range = 1048574,  neg_range = -1048576  giving
+;; (-1048568 -> 1048576).
+
+
 (define_expand "cbranchsi4"
   [(set (pc) (if_then_else
 	      (match_operator 0 "expandable_comparison_operator"
@@ -6947,22 +6954,42 @@
 		      (pc)))]
   "TARGET_32BIT"
   "*
-  if (arm_ccfsm_state == 1 || arm_ccfsm_state == 2)
-    {
-      arm_ccfsm_state += 2;
-      return \"\";
-    }
-  return \"b%d1\\t%l0\";
+     if (arm_ccfsm_state == 1 || arm_ccfsm_state == 2)
+      {
+	arm_ccfsm_state += 2;
+	return \"\";
+      }
+     switch (get_attr_length (insn))
+      {
+	// Thumb2 16-bit b{cond}
+	case 2:
+
+	// Thumb2 32-bit b{cond}
+	case 4: return \"b%d1\\t%l0\";break;
+
+	// Thumb2 b{cond} out of range.  Use unconditional branch.
+	case 8: return arm_gen_far_branch \
+		(operands, 0, \"Lbcond\", \"b%D1\t\");
+	break;
+
+	// A32 b{cond}
+	default: return \"b%d1\\t%l0\";
+      }
   "
   [(set_attr "conds" "use")
    (set_attr "type" "branch")
    (set (attr "length")
-	(if_then_else
-	   (and (match_test "TARGET_THUMB2")
-		(and (ge (minus (match_dup 0) (pc)) (const_int -250))
-		     (le (minus (match_dup 0) (pc)) (const_int 256))))
-	   (const_int 2)
-	   (const_int 4)))]
+	(if_then_else (match_test "TARGET_THUMB2")
+	(if_then_else (and (ge (minus (match_dup 0) (pc)) (const_int -250))
+	(le (minus (match_dup 0) (pc)) (const_int 256)))
+	(const_int 2)
+	(if_then_else (and (ge (minus (match_dup 0) (pc))
+						(const_int -1048568))
+			(le (minus (match_dup 0) (pc)) (const_int 1048576)))
+	(const_int 4)
+	(const_int 8)))
+	(const_int 10)))
+   ]
 )
 
 (define_insn "*arm_cond_branch_reversed"
@@ -6978,17 +7005,36 @@
       arm_ccfsm_state += 2;
       return \"\";
     }
-  return \"b%D1\\t%l0\";
+     switch (get_attr_length (insn))
+      {
+	// Thumb2 16-bit b{cond}
+	case 2:
+
+	// Thumb2 32-bit b{cond}
+	case 4: return \"b%D1\\t%l0\";break;
+
+	// Thumb2 b{cond} out of range.  Use unconditional branch.
+	case 8: return arm_gen_far_branch \
+		(operands, 0, \"Lbcond\", \"b%d1\t\");
+		break;
+	// A32 b{cond}
+	default: return \"b%D1\\t%l0\";
+       }
   "
   [(set_attr "conds" "use")
    (set_attr "type" "branch")
    (set (attr "length")
-	(if_then_else
-	   (and (match_test "TARGET_THUMB2")
-		(and (ge (minus (match_dup 0) (pc)) (const_int -250))
-		     (le (minus (match_dup 0) (pc)) (const_int 256))))
-	   (const_int 2)
-	   (const_int 4)))]
+	(if_then_else (match_test "TARGET_THUMB2")
+	(if_then_else (and (ge (minus (match_dup 0) (pc)) (const_int -250))
+		(le (minus (match_dup 0) (pc)) (const_int 256)))
+	(const_int 2)
+	(if_then_else (and (ge (minus (match_dup 0) (pc))
+							(const_int -1048568))
+		(le (minus (match_dup 0) (pc)) (const_int 1048576)))
+	(const_int 4)
+	(const_int 8)))
+	(const_int 10)))
+   ]
 )
 
 \f
diff --git a/gcc/testsuite/gcc.target/arm/pr91816.c b/gcc/testsuite/gcc.target/arm/pr91816.c
new file mode 100644
index 00000000000..176bf61780b
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/pr91816.c
@@ -0,0 +1,102 @@
+/* { dg-do compile } */
+/* { dg-options "-march=armv7-a -mthumb -mfpu=vfpv3-d16" }  */
+int printf(const char *, ...);
+
+__attribute__((noinline,noclone)) void f1(int a)
+{
+	if (a) {
+#define HW0	printf("Hello World!\n");
+#define HW1	HW0 HW0 HW0 HW0 HW0 HW0 HW0 HW0 HW0 HW0
+#define HW2	HW1 HW1 HW1 HW1 HW1 HW1 HW1 HW1 HW1 HW1
+#define HW3	HW2 HW2 HW2 HW2 HW2 HW2 HW2 HW2 HW2 HW2
+#define HW4	HW3 HW3 HW3 HW3 HW3 HW3 HW3 HW3 HW3 HW3
+#define HW5	HW4 HW4 HW4 HW4 HW4 HW4 HW4 HW4 HW4 HW4
+		HW0
+	}
+}
+
+__attribute__((noinline,noclone)) void f2(int a)
+{
+	if (a) {
+#define HW0	printf("Hello World!\n");
+#define HW1	HW0 HW0 HW0 HW0 HW0 HW0 HW0 HW0 HW0 HW0
+#define HW2	HW1 HW1 HW1 HW1 HW1 HW1 HW1 HW1 HW1 HW1
+#define HW3	HW2 HW2 HW2 HW2 HW2 HW2 HW2 HW2 HW2 HW2
+#define HW4	HW3 HW3 HW3 HW3 HW3 HW3 HW3 HW3 HW3 HW3
+#define HW5	HW4 HW4 HW4 HW4 HW4 HW4 HW4 HW4 HW4 HW4
+		HW3
+	}
+}
+
+
+__attribute__((noinline,noclone)) void f3(int a)
+{
+	if (a) {
+#define HW0	printf("Hello World!\n");
+#define HW1	HW0 HW0 HW0 HW0 HW0 HW0 HW0 HW0 HW0 HW0
+#define HW2	HW1 HW1 HW1 HW1 HW1 HW1 HW1 HW1 HW1 HW1
+#define HW3	HW2 HW2 HW2 HW2 HW2 HW2 HW2 HW2 HW2 HW2
+#define HW4	HW3 HW3 HW3 HW3 HW3 HW3 HW3 HW3 HW3 HW3
+#define HW5	HW4 HW4 HW4 HW4 HW4 HW4 HW4 HW4 HW4 HW4
+		HW5
+	}
+}
+
+__attribute__((noinline,noclone)) void f4(int a)
+{
+	if (a==1) {
+#define HW0	printf("Hello World!\n");
+#define HW1	HW0 HW0 HW0 HW0 HW0 HW0 HW0 HW0 HW0 HW0
+#define HW2	HW1 HW1 HW1 HW1 HW1 HW1 HW1 HW1 HW1 HW1
+#define HW3	HW2 HW2 HW2 HW2 HW2 HW2 HW2 HW2 HW2 HW2
+#define HW4	HW3 HW3 HW3 HW3 HW3 HW3 HW3 HW3 HW3 HW3
+#define HW5	HW4 HW4 HW4 HW4 HW4 HW4 HW4 HW4 HW4 HW4
+		HW0
+	}
+}
+
+__attribute__((noinline,noclone)) void f5(int a)
+{
+	if (a==1) {
+#define HW0	printf("Hello World!\n");
+#define HW1	HW0 HW0 HW0 HW0 HW0 HW0 HW0 HW0 HW0 HW0
+#define HW2	HW1 HW1 HW1 HW1 HW1 HW1 HW1 HW1 HW1 HW1
+#define HW3	HW2 HW2 HW2 HW2 HW2 HW2 HW2 HW2 HW2 HW2
+#define HW4	HW3 HW3 HW3 HW3 HW3 HW3 HW3 HW3 HW3 HW3
+#define HW5	HW4 HW4 HW4 HW4 HW4 HW4 HW4 HW4 HW4 HW4
+		HW3
+	}
+}
+
+
+__attribute__((noinline,noclone)) void f6(int a)
+{
+	if (a==1) {
+#define HW0	printf("Hello World!\n");
+#define HW1	HW0 HW0 HW0 HW0 HW0 HW0 HW0 HW0 HW0 HW0
+#define HW2	HW1 HW1 HW1 HW1 HW1 HW1 HW1 HW1 HW1 HW1
+#define HW3	HW2 HW2 HW2 HW2 HW2 HW2 HW2 HW2 HW2 HW2
+#define HW4	HW3 HW3 HW3 HW3 HW3 HW3 HW3 HW3 HW3 HW3
+#define HW5	HW4 HW4 HW4 HW4 HW4 HW4 HW4 HW4 HW4 HW4
+		HW5
+	}
+}
+
+
+int main(void)
+{
+	f1(0);
+	f2(0);
+	f3(0);
+	f4(0);
+	f5(0);
+	f6(0);
+	return 0;
+}
+
+
+/* { dg-final { scan-assembler-times "beq\\t.L\[0-9\]" 2 } } */
+/* { dg-final { scan-assembler-times "beq\\t.Lbcond\[0-9\]" 1 } } */
+/* { dg-final { scan-assembler-times "bne\\t.L\[0-9\]" 2 } } */
+/* { dg-final { scan-assembler-times "bne\\t.Lbcond\[0-9\]" 1 } } */
+/* { dg-final { scan-assembler-times "b\\t.L\[0-9\]" 2 } } */

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

end of thread, other threads:[~2020-03-04 14:18 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-11 14:57 [PATCH][GCC][ARM] Arm generates out of range conditional branches in Thumb2 (PR91816) Stam Markianos-Wright
2019-10-13 15:37 ` Ramana Radhakrishnan
2019-10-21  9:40   ` Stam Markianos-Wright
2019-11-15 17:27     ` [PING][PATCH][GCC][ARM] " Stam Markianos-Wright
2019-12-02 16:43       ` Stam Markianos-Wright
2019-12-09 17:50         ` Stam Markianos-Wright
2019-12-10 17:03       ` Kyrill Tkachov
2020-01-08 15:19         ` Stam Markianos-Wright
2020-01-16 16:30           ` Stam Markianos-Wright
2020-01-27 16:28             ` [PINGx2][PATCH][GCC][ARM] " Stam Markianos-Wright
2020-01-28 11:05           ` [PING][PATCH][GCC][ARM] " Kyrill Tkachov
2020-01-30 14:55             ` Stam Markianos-Wright
2020-01-30 15:21               ` Kyrill Tkachov
2020-03-04 14:15                 ` Tamar Christina
2020-03-04 14:18                   ` Kyrill Tkachov

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