public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Stam Markianos-Wright <Stam.Markianos-Wright@arm.com>
To: "gcc-patches@gcc.gnu.org" <gcc-patches@gcc.gnu.org>
Cc: "nickc@redhat.com" <nickc@redhat.com>,
	Ramana Radhakrishnan	<Ramana.Radhakrishnan@arm.com>,
	Kyrylo Tkachov <Kyrylo.Tkachov@arm.com>,
	Richard Earnshaw <Richard.Earnshaw@arm.com>
Subject: [PING][PATCH][GCC][ARM] Arm generates out of range conditional branches in Thumb2 (PR91816)
Date: Fri, 15 Nov 2019 17:27:00 -0000	[thread overview]
Message-ID: <9e889eb7-5c5f-3aad-5801-8e972c97856a@arm.com> (raw)
In-Reply-To: <687026c0-05ac-21bc-3b7f-16d138c49134@arm.com>

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

Pinging with more correct maintainers this time :)

Also would need to backport to gcc7,8,9, but need to get this approved 
first!

Thank you,
Stam


-------- Forwarded Message --------
Subject: Re: [PATCH][GCC][ARM] Arm generates out of range conditional 
branches in Thumb2 (PR91816)
Date: Mon, 21 Oct 2019 10:37:09 +0100
From: Stam Markianos-Wright <stam.markianos-wright@arm.com>
To: Ramana Radhakrishnan <ramana.gcc@googlemail.com>
CC: gcc-patches@gcc.gnu.org <gcc-patches@gcc.gnu.org>, nd <nd@arm.com>, 
James Greenhalgh <James.Greenhalgh@arm.com>, Richard Earnshaw 
<Richard.Earnshaw@arm.com>



On 10/13/19 4:23 PM, Ramana Radhakrishnan wrote:
>>
>> 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.
>>
>> 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.
> 
>> 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 *);
>> +
>> +
> 
> Lets get the nits out of the way.
> 
> Unnecessary extra new line, need a space between int and const above.
> 
> 

.Fixed!

>>   #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)
> 
> Not sure if this is some munging from the attachment but check
> vertical alignment of parameters.
> 

.Fixed!

>> +{
>> +  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 "";
>> +}
>> +
>> +
> 
> Unnecessary extra newline.
> 

.Fixed!

>>   #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).
>> +
>> +
> 
> Unnecessary extra newline.
> 

.Fixed!

>>   (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\";
>> +      }
> 
> Please fix indentation here.
> 

.Fixed together with below changes.

>>     "
>>     [(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)))
>> +   ]
> 
> This conditional is unreadable and is getting quite complex.
> 
> Please fix the indentation and add some comments to indicate when
> this is 2, 4, 8, 10 above the pattern and ask for the comment to
> be in sync with this.
> 
> How did we end up with length 10 ? That indicates 2 4 byte instructions
> and a 2 byte instruction ? You are handling lengths 2, 4, 8 above in
> the switch - is length 10 going to be a single A32 b<cond> instruction ?
> 
> What am I missing ?
> 
> 

Ah sorry, I had not realised that the "length" related to the number of 
bytes in the instruction, so I just used it as a variable to then check 
in the switch().
And yes, you are correct in assuming that length 10 would have been the 
A32 b<cond> version.
So the mapping I had in mind was:
2->  Thumb2 b<cond> - narrow 16bit version
4->  Thumb2 b<cond> - wide 32bit version
8->  Thumb2 b       - "far branch".
10-> A32 b<cond>

The new version that maintains the "length=number of bytes" would be:

2->  Thumb2 b<cond> - narrow 16bit version
4->  Thumb2 b<cond> - wide 32bit version OR A32 b<cond>
6->  Thumb2 "far branch" made up from one b<cond> to a very close Lbcond 
label (so 16 bits) and one b for 32 bits. (so 2+4 == 6)

I've gone ahead and done this in the new proposed patch. Let me know if 
it's ok! (also I changed the first check to !TARGET_THUMB2 - this makes 
it slightly more readable). I'm still not sure about this, so any 
suggestions are welcome!

> 
>>   )
>>   
>>   (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)))
>> +   ]
> 
> Same comments as above apply here too.
> 

Same as above.

Thank you for the feedback and apologies for being a clueless :)

And, of course, let me know of any problems or queries!

Cheers,
Stam

> Ramana
> 
>>   )
>>   
>>   \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 } } */
> 



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

diff --git a/gcc/config/arm/arm-protos.h b/gcc/config/arm/arm-protos.h
index f995974f9bb..59ec219da3d 100644
--- a/gcc/config/arm/arm-protos.h
+++ b/gcc/config/arm/arm-protos.h
@@ -570,4 +570,6 @@ 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..7a69ddb6b7b 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -32139,6 +32139,30 @@ 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..7e5e1489214 100644
--- a/gcc/config/arm/arm.md
+++ b/gcc/config/arm/arm.md
@@ -6686,9 +6686,15 @@
 ;; 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"
@@ -6946,23 +6952,56 @@
 		      (label_ref (match_operand 0 "" ""))
 		      (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} or A32 b{cond}.  */
+		case 4: return "b%d1\t%l0";
+			break;
+
+		/* Thumb2 b{cond} out of range.  Use 16-bit b{cond} and
+		   unconditional branch b.  */
+		default: return arm_gen_far_branch \
+				(operands, 0, "Lbcond", "b%D1\t");
+	}
+  }
   [(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")
+
+	;;Target is not Thumb2, therefore is A32.  Generate b{cond}.
+	(const_int 4)
+
+	;; Check if target is within 16-bit Thumb2 b{cond} range.
+	(if_then_else (and (ge (minus (match_dup 0) (pc)) (const_int -250))
+			   (le (minus (match_dup 0) (pc)) (const_int 256)))
+
+		;; Target is Thumb2, within narrow range.
+		;; Generate b{cond}.
+			(const_int 2)
+
+		;; Check if target is within 32-bit Thumb2 b{cond} range.
+			(if_then_else (and (ge (minus (match_dup 0)
+					 (pc))(const_int -1048568))
+					   (le (minus (match_dup 0)
+					 (pc)) (const_int 1048576)))
+
+		;; Target is Thumb2, within wide range.
+		;; Generate b{cond}
+						(const_int 4)
+		;; Target is Thumb2, out of range.
+		;; Generate narrow b{cond} and unconditional branch b.
+						(const_int 6)))))
+  ]
 )
 
 (define_insn "*arm_cond_branch_reversed"
@@ -6972,23 +7011,56 @@
 		      (pc)
 		      (label_ref (match_operand 0 "" ""))))]
   "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} or A32 b{cond}.  */
+		case 4: return "b%D1\t%l0";
+			break;
+
+		/* Thumb2 b{cond} out of range.  Use 16-bit b{cond} and
+		   unconditional branch b.  */
+		default: return arm_gen_far_branch \
+				(operands, 0, "Lbcond", "b%d1\t");
+	}
+  }
   [(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")
+
+	;;Target is not Thumb2, therefore is A32.  Generate b{cond}.
+	(const_int 4)
+
+	;; Check if target is within 16-bit Thumb2 b{cond} range.
+	(if_then_else (and (ge (minus (match_dup 0) (pc)) (const_int -250))
+			   (le (minus (match_dup 0) (pc)) (const_int 256)))
+
+		;; Target is Thumb2, within narrow range.
+		;; Generate b{cond}.
+			(const_int 2)
+
+		;; Check if target is within 32-bit Thumb2 b{cond} range.
+			(if_then_else (and (ge (minus (match_dup 0)
+					 (pc))(const_int -1048568))
+					   (le (minus (match_dup 0)
+					 (pc)) (const_int 1048576)))
+
+		;; Target is Thumb2, within wide range.
+		;; Generate b{cond}.
+						(const_int 4)
+		;; Target is Thumb2, out of range.
+		;; Generate narrow b{cond} and unconditional branch b.
+						(const_int 6)))))
+  ]
 )
 
 \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 } } */



  reply	other threads:[~2019-11-15 17:26 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-11 14:57 [PATCH][GCC][ARM] " Stam Markianos-Wright
2019-10-13 15:37 ` Ramana Radhakrishnan
2019-10-21  9:40   ` Stam Markianos-Wright
2019-11-15 17:27     ` Stam Markianos-Wright [this message]
2019-12-02 16:43       ` [PING][PATCH][GCC][ARM] " 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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=9e889eb7-5c5f-3aad-5801-8e972c97856a@arm.com \
    --to=stam.markianos-wright@arm.com \
    --cc=Kyrylo.Tkachov@arm.com \
    --cc=Ramana.Radhakrishnan@arm.com \
    --cc=Richard.Earnshaw@arm.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=nickc@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).