Re: [PING][PATCH][GCC][ARM] Arm generates out of range conditional branches in Thumb2 (PR91816)
On 3/4/20 2:14 PM, Tamar Christina wrote:
Hi Kyrill,
Ok for backporting this patch to GCC 8 and GCC 9?
Ok assuming bootstrap and test shows no problems.
Thanks,
Kyrill
Thanks,
Tamar
-Original Message-
From: gcc-patches-ow...@gcc.gnu.org
On Behalf Of Kyrill Tkachov
Sent: Thursday, January 30, 2020 14:55
To: Stam Markianos-Wright ; gcc-
patc...@gcc.gnu.org
Cc: ni...@redhat.com; Ramana Radhakrishnan
; Richard Earnshaw
Subject: Re: [PING][PATCH][GCC][ARM] Arm generates out of range
conditional branches in Thumb2 (PR91816)
On 1/30/20 2:42 PM, Stam Markianos-Wright wrote:
On 1/28/20 10:35 AM, Kyrill Tkachov wrote:
Hi Stam,
On 1/8/20 3:18 PM, Stam Markianos-Wright wrote:
On 12/10/19 5:03 PM, Kyrill Tkachov wrote:
Hi Stam,
On 11/15/19 5:26 PM, Stam Markianos-Wright wrote:
Pinging with more correct maintainers this time :)
Also would need to backport to gcc7,8,9, but need to get this
approved first!
Sorry for the delay.
Same here now! Sorry totally forget about this in the lead up to Xmas!
Done the changes marked below and also removed the unnecessary
extra
#defines from the test.
This is ok with a nit on the testcase...
diff --git a/gcc/testsuite/gcc.target/arm/pr91816.c
b/gcc/testsuite/gcc.target/arm/pr91816.c
new file mode 100644
index
..757c897e9c0db32709227b3fdf
1
b4a8033428232
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/pr91816.c
@@ -0,0 +1,61 @@
+/* { dg-do compile } */
+/* { dg-options "-march=armv7-a -mthumb -mfpu=vfpv3-d16" } */ int
+printf(const char *, ...);
+
I think this needs a couple of effective target checks like
arm_hard_vfp_ok and arm_thumb2_ok. See other tests in gcc.target/arm
that add -mthumb to the options.
Hmm, looking back at this now, is there any reason why it can't just be:
/* { dg-do compile } */
/* { dg-require-effective-target arm_thumb2_ok } */
/* { dg-additional-options "-mthumb" } */
were we don't override the march or fpu options at all, but just use
`require-effective-target arm_thumb2_ok` to make sure that thumb2 is
supported?
The attached new diff does just that.
Works for me, there are plenty of configurations run with fpu that it should
get the right coverage.
Ok (make sure commit the updated, if needed, ChangeLog as well)
Thanks!
Kyrill
Cheers :)
Stam.
Thanks,
Kyrill
RE: [PING][PATCH][GCC][ARM] Arm generates out of range conditional branches in Thumb2 (PR91816)
Hi Kyrill,
Ok for backporting this patch to GCC 8 and GCC 9?
Thanks,
Tamar
> -Original Message-
> From: gcc-patches-ow...@gcc.gnu.org
> On Behalf Of Kyrill Tkachov
> Sent: Thursday, January 30, 2020 14:55
> To: Stam Markianos-Wright ; gcc-
> patc...@gcc.gnu.org
> Cc: ni...@redhat.com; Ramana Radhakrishnan
> ; Richard Earnshaw
>
> Subject: Re: [PING][PATCH][GCC][ARM] Arm generates out of range
> conditional branches in Thumb2 (PR91816)
>
>
> On 1/30/20 2:42 PM, Stam Markianos-Wright wrote:
> >
> >
> > On 1/28/20 10:35 AM, Kyrill Tkachov wrote:
> >> Hi Stam,
> >>
> >> On 1/8/20 3:18 PM, Stam Markianos-Wright wrote:
> >>>
> >>> On 12/10/19 5:03 PM, Kyrill Tkachov wrote:
> >>>> Hi Stam,
> >>>>
> >>>> On 11/15/19 5:26 PM, Stam Markianos-Wright wrote:
> >>>>> Pinging with more correct maintainers this time :)
> >>>>>
> >>>>> Also would need to backport to gcc7,8,9, but need to get this
> >>>>> approved first!
> >>>>>
> >>>> Sorry for the delay.
> >>> Same here now! Sorry totally forget about this in the lead up to Xmas!
> >>>
> >>> Done the changes marked below and also removed the unnecessary
> extra
> >>> #defines from the test.
> >>
> >>
> >> This is ok with a nit on the testcase...
> >>
> >>
> >> diff --git a/gcc/testsuite/gcc.target/arm/pr91816.c
> >> b/gcc/testsuite/gcc.target/arm/pr91816.c
> >> new file mode 100644
> >> index
> >>
> ..757c897e9c0db32709227b3fdf
> 1
> >> b4a8033428232
> >> --- /dev/null
> >> +++ b/gcc/testsuite/gcc.target/arm/pr91816.c
> >> @@ -0,0 +1,61 @@
> >> +/* { dg-do compile } */
> >> +/* { dg-options "-march=armv7-a -mthumb -mfpu=vfpv3-d16" } */ int
> >> +printf(const char *, ...);
> >> +
> >>
> >> I think this needs a couple of effective target checks like
> >> arm_hard_vfp_ok and arm_thumb2_ok. See other tests in gcc.target/arm
> >> that add -mthumb to the options.
> >
> > Hmm, looking back at this now, is there any reason why it can't just be:
> >
> > /* { dg-do compile } */
> > /* { dg-require-effective-target arm_thumb2_ok } */
> > /* { dg-additional-options "-mthumb" } */
> >
> > were we don't override the march or fpu options at all, but just use
> > `require-effective-target arm_thumb2_ok` to make sure that thumb2 is
> > supported?
> >
> > The attached new diff does just that.
> >
>
> Works for me, there are plenty of configurations run with fpu that it should
> get the right coverage.
>
> Ok (make sure commit the updated, if needed, ChangeLog as well)
>
> Thanks!
>
> Kyrill
>
>
> > Cheers :)
> >
> > Stam.
> >
> >>
> >> Thanks,
> >> Kyrill
> >>
> >
Re: [PING][PATCH][GCC][ARM] Arm generates out of range conditional branches in Thumb2 (PR91816)
On 1/30/20 2:42 PM, Stam Markianos-Wright wrote:
On 1/28/20 10:35 AM, Kyrill Tkachov wrote:
Hi Stam,
On 1/8/20 3:18 PM, Stam Markianos-Wright wrote:
On 12/10/19 5:03 PM, Kyrill Tkachov wrote:
Hi Stam,
On 11/15/19 5:26 PM, Stam Markianos-Wright wrote:
Pinging with more correct maintainers this time :)
Also would need to backport to gcc7,8,9, but need to get this
approved
first!
Sorry for the delay.
Same here now! Sorry totally forget about this in the lead up to Xmas!
Done the changes marked below and also removed the unnecessary extra
#defines
from the test.
This is ok with a nit on the testcase...
diff --git a/gcc/testsuite/gcc.target/arm/pr91816.c
b/gcc/testsuite/gcc.target/arm/pr91816.c
new file mode 100644
index
..757c897e9c0db32709227b3fdf1b4a8033428232
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/pr91816.c
@@ -0,0 +1,61 @@
+/* { dg-do compile } */
+/* { dg-options "-march=armv7-a -mthumb -mfpu=vfpv3-d16" } */
+int printf(const char *, ...);
+
I think this needs a couple of effective target checks like
arm_hard_vfp_ok and arm_thumb2_ok. See other tests in gcc.target/arm
that add -mthumb to the options.
Hmm, looking back at this now, is there any reason why it can't just be:
/* { dg-do compile } */
/* { dg-require-effective-target arm_thumb2_ok } */
/* { dg-additional-options "-mthumb" } */
were we don't override the march or fpu options at all, but just use
`require-effective-target arm_thumb2_ok` to make sure that thumb2 is
supported?
The attached new diff does just that.
Works for me, there are plenty of configurations run with fpu that it
should get the right coverage.
Ok (make sure commit the updated, if needed, ChangeLog as well)
Thanks!
Kyrill
Cheers :)
Stam.
Thanks,
Kyrill
Re: [PING][PATCH][GCC][ARM] Arm generates out of range conditional branches in Thumb2 (PR91816)
On 1/28/20 10:35 AM, Kyrill Tkachov wrote:
Hi Stam,
On 1/8/20 3:18 PM, Stam Markianos-Wright wrote:
On 12/10/19 5:03 PM, Kyrill Tkachov wrote:
Hi Stam,
On 11/15/19 5:26 PM, Stam Markianos-Wright wrote:
Pinging with more correct maintainers this time :)
Also would need to backport to gcc7,8,9, but need to get this approved
first!
Sorry for the delay.
Same here now! Sorry totally forget about this in the lead up to Xmas!
Done the changes marked below and also removed the unnecessary extra #defines
from the test.
This is ok with a nit on the testcase...
diff --git a/gcc/testsuite/gcc.target/arm/pr91816.c
b/gcc/testsuite/gcc.target/arm/pr91816.c
new file mode 100644
index
..757c897e9c0db32709227b3fdf1b4a8033428232
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/pr91816.c
@@ -0,0 +1,61 @@
+/* { dg-do compile } */
+/* { dg-options "-march=armv7-a -mthumb -mfpu=vfpv3-d16" } */
+int printf(const char *, ...);
+
I think this needs a couple of effective target checks like arm_hard_vfp_ok and
arm_thumb2_ok. See other tests in gcc.target/arm that add -mthumb to the options.
Hmm, looking back at this now, is there any reason why it can't just be:
/* { dg-do compile } */
/* { dg-require-effective-target arm_thumb2_ok } */
/* { dg-additional-options "-mthumb" } */
were we don't override the march or fpu options at all, but just use
`require-effective-target arm_thumb2_ok` to make sure that thumb2 is supported?
The attached new diff does just that.
Cheers :)
Stam.
Thanks,
Kyrill
diff --git a/gcc/config/arm/arm-protos.h b/gcc/config/arm/arm-protos.h
index 7c4b1003844..8895becc639 100644
--- a/gcc/config/arm/arm-protos.h
+++ b/gcc/config/arm/arm-protos.h
@@ -576,4 +576,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 07231d722b9..ee5de169f3e 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -32626,6 +32626,40 @@ arm_run_selftests (void)
}
} /* Namespace selftest. */
+
+/* Generate code to enable conditional branches in functions over 1 MiB.
+ Parameters are:
+ operands: is the operands list of the asm insn (see arm_cond_branch or
+ arm_cond_branch_reversed).
+ pos_label: is an index into the operands array where operands[pos_label]
is
+ the asm label of the final jump destination.
+ dest: is a string which is used to generate the asm label of the
intermediate
+ destination
+ branch_format: is a string denoting the intermediate branch format, e.g.
+ "beq", "bne", etc. */
+
+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 f89a2d412df..fb1d4547e5c 100644
--- a/gcc/config/arm/arm.md
+++ b/gcc/config/arm/arm.md
@@ -7546,9 +7546,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' 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' pos_range = 1048574, neg_range = -1048576 giving
+;; (-1048568 -> 1048576).
+
(define_expand "cbranchsi4"
[(set (pc) (if_then_else
(match_operator 0 "expandable_comparison_operator"
@@ -7721,23 +7727,50 @@
(label_ref (match_operand 0 "" ""))
(pc)))]
"TARGET_32BIT"
- "*
- if (arm_ccfsm_state == 1 || arm_ccfsm_state == 2)
+ {
+if (arm_ccfsm_state == 1 || arm_ccfsm_state == 2)
{
arm_ccfsm_state += 2;
- return \"\";
+ return "";
}
- return \"b%d1\\t%l0\";
- "
+switch (get_attr_length (insn))
+ {
+ case 2: /* Thumb2 16-bit b{cond}. */
+ case 4: /* Thumb2 32-bit b{cond} or A32 b{cond}.
Re: [PING][PATCH][GCC][ARM] Arm generates out of range conditional branches in Thumb2 (PR91816)
Hi Stam,
On 1/8/20 3:18 PM, Stam Markianos-Wright wrote:
On 12/10/19 5:03 PM, Kyrill Tkachov wrote:
Hi Stam,
On 11/15/19 5:26 PM, Stam Markianos-Wright wrote:
Pinging with more correct maintainers this time :)
Also would need to backport to gcc7,8,9, but need to get this approved
first!
Sorry for the delay.
Same here now! Sorry totally forget about this in the lead up to Xmas!
Done the changes marked below and also removed the unnecessary extra #defines
from the test.
This is ok with a nit on the testcase...
diff --git a/gcc/testsuite/gcc.target/arm/pr91816.c
b/gcc/testsuite/gcc.target/arm/pr91816.c
new file mode 100644
index
..757c897e9c0db32709227b3fdf1b4a8033428232
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/pr91816.c
@@ -0,0 +1,61 @@
+/* { dg-do compile } */
+/* { dg-options "-march=armv7-a -mthumb -mfpu=vfpv3-d16" } */
+int printf(const char *, ...);
+
I think this needs a couple of effective target checks like arm_hard_vfp_ok and
arm_thumb2_ok. See other tests in gcc.target/arm that add -mthumb to the
options.
Thanks,
Kyrill
[PINGx2][PATCH][GCC][ARM] Arm generates out of range conditional branches in Thumb2 (PR91816)
On 1/16/20 4:06 PM, Stam Markianos-Wright wrote:
>
>
> On 1/8/20 3:18 PM, Stam Markianos-Wright wrote:
>>
>>
>> On 12/10/19 5:03 PM, Kyrill Tkachov wrote:
>>> Hi Stam,
>>>
>>> On 11/15/19 5:26 PM, Stam Markianos-Wright wrote:
>>>> Pinging with more correct maintainers this time :)
>>>>
>>>> Also would need to backport to gcc7,8,9, but need to get this approved
>>>> first!
>>>>
>>>
>>> Sorry for the delay.
>>
>> Same here now! Sorry totally forget about this in the lead up to Xmas!
>>
>> Done the changes marked below and also removed the unnecessary extra
>> #defines
>> from the test.
>
> Ping :)
>
> Cheers,
> Stam
>
>>
>>>
>>>
>>>> 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
>>>> To: Ramana Radhakrishnan
>>>> CC: gcc-patches@gcc.gnu.org , nd ,
>>>> James Greenhalgh , Richard Earnshaw
>>>>
>>>>
>>>>
>>>>
>>>> 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
>>>> >>
>>>> >> * config/arm/arm.md: Update b 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
>>>> >>
>>>> >> * 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 , \
>>>> >&g
Re: [PING][PATCH][GCC][ARM] Arm generates out of range conditional branches in Thumb2 (PR91816)
On 1/8/20 3:18 PM, Stam Markianos-Wright wrote:
>
>
> On 12/10/19 5:03 PM, Kyrill Tkachov wrote:
>> Hi Stam,
>>
>> On 11/15/19 5:26 PM, Stam Markianos-Wright wrote:
>>> Pinging with more correct maintainers this time :)
>>>
>>> Also would need to backport to gcc7,8,9, but need to get this approved
>>> first!
>>>
>>
>> Sorry for the delay.
>
> Same here now! Sorry totally forget about this in the lead up to Xmas!
>
> Done the changes marked below and also removed the unnecessary extra #defines
> from the test.
Ping :)
Cheers,
Stam
>
>>
>>
>>> 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
>>> To: Ramana Radhakrishnan
>>> CC: gcc-patches@gcc.gnu.org , nd ,
>>> James Greenhalgh , Richard Earnshaw
>>>
>>>
>>>
>>>
>>> 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
>>> >>
>>> >> * config/arm/arm.md: Update b 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
>>> >>
>>> >> * 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,
Re: [PING][PATCH][GCC][ARM] Arm generates out of range conditional branches in Thumb2 (PR91816)
On 12/10/19 5:03 PM, Kyrill Tkachov wrote:
> Hi Stam,
>
> On 11/15/19 5:26 PM, Stam Markianos-Wright wrote:
>> Pinging with more correct maintainers this time :)
>>
>> Also would need to backport to gcc7,8,9, but need to get this approved
>> first!
>>
>
> Sorry for the delay.
Same here now! Sorry totally forget about this in the lead up to Xmas!
Done the changes marked below and also removed the unnecessary extra #defines
from the test.
>
>
>> 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
>> To: Ramana Radhakrishnan
>> CC: gcc-patches@gcc.gnu.org , nd ,
>> James Greenhalgh , Richard Earnshaw
>>
>>
>>
>>
>> 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
>> >>
>> >> * config/arm/arm.md: Update b 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
>> >>
>> >> * 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
>>
Re: [PING][PATCH][GCC][ARM] Arm generates out of range conditional branches in Thumb2 (PR91816)
Hi Stam,
On 11/15/19 5:26 PM, Stam Markianos-Wright wrote:
Pinging with more correct maintainers this time :)
Also would need to backport to gcc7,8,9, but need to get this approved
first!
Sorry for the delay.
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
To: Ramana Radhakrishnan
CC: gcc-patches@gcc.gnu.org , nd ,
James Greenhalgh , Richard Earnshaw
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
>>
>> * config/arm/arm.md: Update b 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
>>
>> * 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' 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' 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 (
[PING][PATCH][GCC][ARM] Arm generates out of range conditional branches in Thumb2 (PR91816)
On 12/2/19 4:43 PM, Stam Markianos-Wright wrote:
>
>
> On 11/15/19 5:26 PM, Stam Markianos-Wright wrote:
>> 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
>> To: Ramana Radhakrishnan
>> CC: gcc-patches@gcc.gnu.org , nd
>> , James Greenhalgh , Richard
>> Earnshaw
>>
>>
>>
>> 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
>>>>
>>>> * config/arm/arm.md: Update b 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
>>>>
>>>> * 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 +
[PING][PATCH][GCC][ARM] Arm generates out of range conditional branches in Thumb2 (PR91816)
On 11/15/19 5:26 PM, Stam Markianos-Wright wrote:
> 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
> To: Ramana Radhakrishnan
> CC: gcc-patches@gcc.gnu.org , nd ,
> James Greenhalgh , Richard Earnshaw
>
>
>
>
> 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
>>>
>>> * config/arm/arm.md: Update b 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
>>>
>>> * 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' pos_range = 254, neg_range = -256 giving (-250
>>> ->256).
>>> +;; In 32-bit Thumb these ranges are:
>>> +;; For a 'b' +/- 16MB is not
[PING][PATCH][GCC][ARM] Arm generates out of range conditional branches in Thumb2 (PR91816)
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
To: Ramana Radhakrishnan
CC: gcc-patches@gcc.gnu.org , nd ,
James Greenhalgh , Richard Earnshaw
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
>>
>> * config/arm/arm.md: Update b 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
>>
>> * 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' 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' 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)
>> -{
&
Re: [PATCH][GCC][ARM] Arm generates out of range conditional branches in Thumb2 (PR91816)
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
>>
>> * config/arm/arm.md: Update b 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
>>
>> * 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' 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' 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
Re: [PATCH][GCC][ARM] Arm generates out of range conditional branches in Thumb2 (PR91816)
>
> 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
>
> * config/arm/arm.md: Update b 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
>
> * 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.
> #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.
> +{
> + 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.
> #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' 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' pos_range = 1048574, neg_range = -1048576 giving
> +;; (-1048568 -> 1048576).
> +
> +
Unnecessary extra newline.
> (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.
>"
>[(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))
[PATCH][GCC][ARM] Arm generates out of range conditional branches in Thumb2 (PR91816)
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
* config/arm/arm.md: Update b 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
* 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 *);
+
+
#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' 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' 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
