Patchwork Fix test case failure reported in PR54989

login
register
mail settings
Submitter Bin Cheng
Date Oct. 22, 2012, 8:15 a.m.
Message ID <002101cdb02d$5a6ca960$0f45fc20$@cheng@arm.com>
Download mbox | patch
Permalink /patch/193106/
State New
Headers show

Comments

Bin Cheng - Oct. 22, 2012, 8:15 a.m.
> -----Original Message-----
> From: Jakub Jelinek [mailto:jakub@redhat.com]
> Sent: Monday, October 22, 2012 3:16 PM
> To: Bin Cheng
> Cc: gcc-patches@gcc.gnu.org
> Subject: Re: [PATCH GCC]Fix test case failure reported in PR54989
> 
> On Mon, Oct 22, 2012 at 11:00:08AM +0800, Bin Cheng wrote:
> > The test case "gcc/testsuite/gcc.dg/hoist-register-pressure.c" is
> > failed on x86_64-apple-darwin because it uses more registers than
> > x86_64-linux. This can be fixed by simplifying the case using fewer
> registers.
> >
> > Tested on x86_64-apple-darwin/x86_64-linux, is it OK?
> 
> I'd say it is better to do the scan-rtl-dump only on nonpic targets, that
way
> it won't be done on darwin or for testing with --target_board=unix/-fpic
where
> it would fail too.  You can add the test with smaller register pressure as
a
> new test (hoist-register-pressure2.c).
> 

Hi Jakub,
Thanks for the suggestion. Updated patch as attached. Now it can work with
-fPIC/-fno-PIC options.
Tested on x86_64-apple-darwin/x86-linux, is it OK.

Thanks

gcc/testsuite/ChangeLog
2012-10-22  Bin Cheng  <bin.cheng@arm.com>

	* gcc.dg/hoist-register-pressure-1.c: Rename from
	hoist-register-pressure.c. Add nonpic condition.
	* gcc.dg/hoist-register-pressure-2.c: New test.
Jeff Law - Oct. 26, 2012, 6:53 p.m.
On 10/22/2012 02:15 AM, Bin Cheng wrote:
>
>
>> -----Original Message-----
>> From: Jakub Jelinek [mailto:jakub@redhat.com]
>> Sent: Monday, October 22, 2012 3:16 PM
>> To: Bin Cheng
>> Cc: gcc-patches@gcc.gnu.org
>> Subject: Re: [PATCH GCC]Fix test case failure reported in PR54989
>>
>> On Mon, Oct 22, 2012 at 11:00:08AM +0800, Bin Cheng wrote:
>>> The test case "gcc/testsuite/gcc.dg/hoist-register-pressure.c" is
>>> failed on x86_64-apple-darwin because it uses more registers than
>>> x86_64-linux. This can be fixed by simplifying the case using fewer
>> registers.
>>>
>>> Tested on x86_64-apple-darwin/x86_64-linux, is it OK?
>>
>> I'd say it is better to do the scan-rtl-dump only on nonpic targets, that
> way
>> it won't be done on darwin or for testing with --target_board=unix/-fpic
> where
>> it would fail too.  You can add the test with smaller register pressure as
> a
>> new test (hoist-register-pressure2.c).
>>
>
> Hi Jakub,
> Thanks for the suggestion. Updated patch as attached. Now it can work with
> -fPIC/-fno-PIC options.
> Tested on x86_64-apple-darwin/x86-linux, is it OK.
>
> Thanks
>
> gcc/testsuite/ChangeLog
> 2012-10-22  Bin Cheng  <bin.cheng@arm.com>
>
> 	* gcc.dg/hoist-register-pressure-1.c: Rename from
> 	hoist-register-pressure.c. Add nonpic condition.
> 	* gcc.dg/hoist-register-pressure-2.c: New test.
This is fine.  Sorry for the delay.

jeff
Bin Cheng - Oct. 27, 2012, 4:14 a.m.
> -----Original Message-----
> From: Jeff Law [mailto:law@redhat.com]
> Sent: Saturday, October 27, 2012 2:54 AM
> To: Bin Cheng
> Cc: 'Jakub Jelinek'; gcc-patches@gcc.gnu.org
> Subject: Re: [PATCH GCC]Fix test case failure reported in PR54989
> 
> On 10/22/2012 02:15 AM, Bin Cheng wrote:
> >
> >
> >> -----Original Message-----
> >> From: Jakub Jelinek [mailto:jakub@redhat.com]
> >> Sent: Monday, October 22, 2012 3:16 PM
> >> To: Bin Cheng
> >> Cc: gcc-patches@gcc.gnu.org
> >> Subject: Re: [PATCH GCC]Fix test case failure reported in PR54989
> >>
> >> On Mon, Oct 22, 2012 at 11:00:08AM +0800, Bin Cheng wrote:
> >>> The test case "gcc/testsuite/gcc.dg/hoist-register-pressure.c" is
> >>> failed on x86_64-apple-darwin because it uses more registers than
> >>> x86_64-linux. This can be fixed by simplifying the case using fewer
> >> registers.
> >>>
> >>> Tested on x86_64-apple-darwin/x86_64-linux, is it OK?
> >>
> >> I'd say it is better to do the scan-rtl-dump only on nonpic targets,
> >> that
> > way
> >> it won't be done on darwin or for testing with
> >> --target_board=unix/-fpic
> > where
> >> it would fail too.  You can add the test with smaller register
> >> pressure as
> > a
> >> new test (hoist-register-pressure2.c).
> >>
> >
> > Hi Jakub,
> > Thanks for the suggestion. Updated patch as attached. Now it can work
> > with -fPIC/-fno-PIC options.
> > Tested on x86_64-apple-darwin/x86-linux, is it OK.
> >
> > Thanks
> >
> > gcc/testsuite/ChangeLog
> > 2012-10-22  Bin Cheng  <bin.cheng@arm.com>
> >
> > 	* gcc.dg/hoist-register-pressure-1.c: Rename from
> > 	hoist-register-pressure.c. Add nonpic condition.
> > 	* gcc.dg/hoist-register-pressure-2.c: New test.
> This is fine.  Sorry for the delay.
> 

Thanks Jeff.
Actually I am going to submit a patch monitoring the change of register
pressure accurately as we discussed before. With that patch, expressions
will be hoisted anyway if it can decrease register pressure thus this test
case won't fail anymore. So I am wondering whether I should check in this
patch or just waiting for that patch to fix this failure?
What's your opinion?

Thanks.
Jeff Law - Oct. 29, 2012, 6:26 p.m.
On 10/26/2012 10:14 PM, Bin Cheng wrote:
> Actually I am going to submit a patch monitoring the change of register
> pressure accurately as we discussed before. With that patch, expressions
> will be hoisted anyway if it can decrease register pressure thus this test
> case won't fail anymore. So I am wondering whether I should check in this
> patch or just waiting for that patch to fix this failure?
> What's your opinion?
I'd go ahead and get in the fix for this failure as we would prefer as 
few unexpected failures as possible.

jeff
Bin Cheng - Oct. 30, 2012, 2:18 a.m.
> -----Original Message-----
> From: Jeff Law [mailto:law@redhat.com]
> Sent: Tuesday, October 30, 2012 2:27 AM
> To: Bin Cheng
> Cc: 'Jakub Jelinek'; gcc-patches@gcc.gnu.org
> Subject: Re: [PATCH GCC]Fix test case failure reported in PR54989
> 
> On 10/26/2012 10:14 PM, Bin Cheng wrote:
> > Actually I am going to submit a patch monitoring the change of
> > register pressure accurately as we discussed before. With that patch,
> > expressions will be hoisted anyway if it can decrease register
> > pressure thus this test case won't fail anymore. So I am wondering
> > whether I should check in this patch or just waiting for that patch to
fix
> this failure?
> > What's your opinion?
> I'd go ahead and get in the fix for this failure as we would prefer as few
> unexpected failures as possible.

Committed as r192976.

Thanks.

Patch

Index: gcc/testsuite/gcc.dg/hoist-register-pressure-1.c
===================================================================
--- gcc/testsuite/gcc.dg/hoist-register-pressure-1.c	(revision 0)
+++ gcc/testsuite/gcc.dg/hoist-register-pressure-1.c	(revision 0)
@@ -0,0 +1,31 @@ 
+/* { dg-options "-Os -fdump-rtl-hoist" }  */
+/* { dg-final { scan-rtl-dump "PRE/HOIST: end of bb .* copying expression" "hoist" { target { nonpic } } } } */
+
+#define BUF 100
+int a[BUF];
+
+void com (int);
+void bar (int);
+
+int foo (int x, int y, int z)
+{
+  /* "x+y" won't be hoisted if "-fira-hoist-pressure" is disabled,
+     because its rtx_cost is too small.  */
+  if (z)
+    {
+      a[1] = a[0] + a[2];
+      a[2] = a[1] + a[3];
+      a[3] = a[2] + a[4];
+      a[4] = a[3] + a[5];
+      a[5] = a[4] + a[6];
+      a[6] = a[5] + a[7];
+      a[7] = a[6] + a[8];
+      com (x+y);
+    }
+  else
+    {
+      bar (x+y);
+    }
+
+  return 0;
+}
Index: gcc/testsuite/gcc.dg/hoist-register-pressure-2.c
===================================================================
--- gcc/testsuite/gcc.dg/hoist-register-pressure-2.c	(revision 0)
+++ gcc/testsuite/gcc.dg/hoist-register-pressure-2.c	(revision 0)
@@ -0,0 +1,32 @@ 
+/* { dg-options "-Os -fdump-rtl-hoist" }  */
+/* { dg-final { scan-rtl-dump "PRE/HOIST: end of bb .* copying expression" "hoist" } } */
+
+#define BUF 100
+int a[BUF];
+
+void com (int);
+void bar (int);
+
+int foo (int x, int y, int z)
+{
+  /* "x+y" won't be hoisted if "-fira-hoist-pressure" is disabled,
+     because its rtx_cost is too small.  */
+  if (z)
+    {
+      a[1] = a[0];
+      a[2] = a[1];
+      a[3] = a[3];
+      a[4] = a[5];
+      a[5] = a[7];
+      a[6] = a[11];
+      a[7] = a[13];
+      a[8] = a[17];
+      com (x+y);
+    }
+  else
+    {
+      bar (x+y);
+    }
+
+  return 0;
+}
Index: gcc/testsuite/gcc.dg/hoist-register-pressure.c
===================================================================
--- gcc/testsuite/gcc.dg/hoist-register-pressure.c	(revision 192629)
+++ gcc/testsuite/gcc.dg/hoist-register-pressure.c	(working copy)
@@ -1,31 +0,0 @@ 
-/* { dg-options "-Os -fdump-rtl-hoist" }  */
-/* { dg-final { scan-rtl-dump "PRE/HOIST: end of bb .* copying expression" "hoist" } } */
-
-#define BUF 100
-int a[BUF];
-
-void com (int);
-void bar (int);
-
-int foo (int x, int y, int z)
-{
-  /* "x+y" won't be hoisted if "-fira-hoist-pressure" is disabled,
-     because its rtx_cost is too small.  */
-  if (z)
-    {
-      a[1] = a[0] + a[2];
-      a[2] = a[1] + a[3];
-      a[3] = a[2] + a[4];
-      a[4] = a[3] + a[5];
-      a[5] = a[4] + a[6];
-      a[6] = a[5] + a[7];
-      a[7] = a[6] + a[8];
-      com (x+y);
-    }
-  else
-    {
-      bar (x+y);
-    }
-
-  return 0;
-}