| Message ID | 002101cdb02d$5a6ca960$0f45fc20$@cheng@arm.com |
|---|---|
| State | New |
| Headers | show |
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
> -----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.
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
> -----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.
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; -}