Patchwork [ARM] Fix PR44557 (Thumb-1 ICE)

login
register
mail settings
Submitter Chung-Lin Tang
Date Sept. 26, 2012, 8:58 a.m.
Message ID <5062C3A7.7010008@codesourcery.com>
Download mbox | patch
Permalink /patch/187015/
State New
Headers show

Comments

Chung-Lin Tang - Sept. 26, 2012, 8:58 a.m.
There was a patch of mine here from over an year ago:
http://gcc.gnu.org/ml/gcc-patches/2011-01/msg01846.html

That old patch contained a constraint modification that was a point of
discussion at the time, but upon re-testing I found it was unneeded for
fixing the PR (don't remember why I included it)

Resubmitting the patch adapted, basically a single-liner adding
CORE_REGS to the preferred reload class under Thumb-1. Cross-tested
with no regressions under "-march=armv5te -mthumb". Okay for trunk?

Thanks,
Chung-Lin

2012-09-26  Chung-Lin Tang  <cltang@codesourcery.com>

	PR target/44557
	* config/arm/arm.c (arm_preferred_reload_class): Add CORE_REGS
	to Thumb-1 case.

	testsuite/
	PR target/44557
	* gcc.target/arm/pr44557.c: New test.
Janis Johnson - Sept. 26, 2012, 10:25 p.m.
On 09/26/2012 01:58 AM, Chung-Lin Tang wrote:

+/* { dg-do compile } */
+/* { dg-options "-mthumb -O1 -march=armv5te -fno-omit-frame-pointer -fno-forward-propagate" }  */
+/* { dg-require-effective-target arm_thumb1_ok } */

This test will fail to compile for test flags that conflict with
the -march option, and the specified -march option might be
overridden with similar options from other test flags.  The problem
might have also been seen for other -march options.  I recommend
leaving it off and omitting the dg-require so the test can be run
for more multilibs.

Janis
Chung-Lin Tang - Oct. 16, 2012, 9:25 a.m.
On 12/9/27 6:25 AM, Janis Johnson wrote:
> On 09/26/2012 01:58 AM, Chung-Lin Tang wrote:
> 
> +/* { dg-do compile } */
> +/* { dg-options "-mthumb -O1 -march=armv5te -fno-omit-frame-pointer -fno-forward-propagate" }  */
> +/* { dg-require-effective-target arm_thumb1_ok } */
> 
> This test will fail to compile for test flags that conflict with
> the -march option, and the specified -march option might be
> overridden with similar options from other test flags.  The problem
> might have also been seen for other -march options.  I recommend
> leaving it off and omitting the dg-require so the test can be run
> for more multilibs.

I'm not sure, as the intent is to test a Thumb-1 case here. If the
maintainers think we should adjust the testcase, I'm of course fine with it.

And ping for the patch.

Thanks,
Chung-Lin
Ramana Radhakrishnan - Oct. 20, 2012, 6:41 a.m.
On Tue, Oct 16, 2012 at 10:25 AM, Chung-Lin Tang
<cltang@codesourcery.com> wrote:
> On 12/9/27 6:25 AM, Janis Johnson wrote:
>> On 09/26/2012 01:58 AM, Chung-Lin Tang wrote:
>>
>> +/* { dg-do compile } */
>> +/* { dg-options "-mthumb -O1 -march=armv5te -fno-omit-frame-pointer -fno-forward-propagate" }  */
>> +/* { dg-require-effective-target arm_thumb1_ok } */
>>
>> This test will fail to compile for test flags that conflict with
>> the -march option, and the specified -march option might be
>> overridden with similar options from other test flags.  The problem
>> might have also been seen for other -march options.  I recommend
>> leaving it off and omitting the dg-require so the test can be run
>> for more multilibs.
>
> I'm not sure, as the intent is to test a Thumb-1 case here. If the
> maintainers think we should adjust the testcase, I'm of course fine with it.

I think this is OK but you need to prune out the conflict warnings to
reduce annoyance for folks doing multilib testing and it does look
like more than one group.

Longer term I wonder if we should reorganise gcc.target/arm and indeed
gcc.target/aarch64 . Janis, do you have any other ideas ?

* to contain a torture script that goes through all combinations of
architectures and fpus' / arm / thumb for all the tests.
* another sub-level directory for such directed tests where multilib
options aren't applied which are essentially from regressions.

However I don't know of an easy way by which we can ignore said
multilib flags ?

Ramana


>
> And ping for the patch.
>
> Thanks,
> Chung-Lin
>
Janis Johnson - Oct. 20, 2012, 4:32 p.m.
On 10/19/2012 11:41 PM, Ramana Radhakrishnan wrote:
> On Tue, Oct 16, 2012 at 10:25 AM, Chung-Lin Tang
> <cltang@codesourcery.com> wrote:
>> On 12/9/27 6:25 AM, Janis Johnson wrote:
>>> On 09/26/2012 01:58 AM, Chung-Lin Tang wrote:
>>>
>>> +/* { dg-do compile } */
>>> +/* { dg-options "-mthumb -O1 -march=armv5te -fno-omit-frame-pointer -fno-forward-propagate" }  */
>>> +/* { dg-require-effective-target arm_thumb1_ok } */
>>>
>>> This test will fail to compile for test flags that conflict with
>>> the -march option, and the specified -march option might be
>>> overridden with similar options from other test flags.  The problem
>>> might have also been seen for other -march options.  I recommend
>>> leaving it off and omitting the dg-require so the test can be run
>>> for more multilibs.
>>
>> I'm not sure, as the intent is to test a Thumb-1 case here. If the
>> maintainers think we should adjust the testcase, I'm of course fine with it.
> 
> I think this is OK but you need to prune out the conflict warnings to
> reduce annoyance for folks doing multilib testing and it does look
> like more than one group.
> 
> Longer term I wonder if we should reorganise gcc.target/arm and indeed
> gcc.target/aarch64 . Janis, do you have any other ideas ?
> 
> * to contain a torture script that goes through all combinations of
> architectures and fpus' / arm / thumb for all the tests.
> * another sub-level directory for such directed tests where multilib
> options aren't applied which are essentially from regressions.
> 
> However I don't know of an easy way by which we can ignore said
> multilib flags ?
> 
> Ramana

Multilib flags are added deep in DejaGnu, and we would need to have a
local copy of a large procedure in order to do that.

Do enough people run a default multilib that we could use a torture
script only for a multilib with no flags?

Janis

Patch

Index: config/arm/arm.c
===================================================================
--- config/arm/arm.c	(revision 191753)
+++ config/arm/arm.c	(working copy)
@@ -6254,6 +6254,7 @@  arm_preferred_reload_class (rtx x ATTRIBUTE_UNUSED
   else
     {
       if (rclass == GENERAL_REGS
+	  || rclass == CORE_REGS
 	  || rclass == HI_REGS
 	  || rclass == NO_REGS
 	  || rclass == STACK_REG)
Index: testsuite/gcc.target/arm/pr44557.c
===================================================================
--- testsuite/gcc.target/arm/pr44557.c	(revision 0)
+++ testsuite/gcc.target/arm/pr44557.c	(revision 0)
@@ -0,0 +1,26 @@ 
+/* { dg-do compile } */
+/* { dg-options "-mthumb -O1 -march=armv5te -fno-omit-frame-pointer -fno-forward-propagate" }  */
+/* { dg-require-effective-target arm_thumb1_ok } */
+
+struct S
+{
+  short x, y;
+};
+
+void foo (struct S *p, struct S *q, char *t, int n)
+{
+  struct S *c, d;
+  int x = 1;
+
+  while (n--)
+    {
+      if (*t && p)
+      c = p;
+      q->x = d.x + c->x + c->y;
+      if (x)
+      {
+        x = 0;
+	   d.x += c->x;
+	   }
+    }
+}