[PATCH] Fix PR rtl-optimization/71634

[PATCH] Fix PR rtl-optimization/71634

[Martin Liška]

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

Hello.

Following patch changes minimum of ira-max-loops-num to 1.
Having the minimum equal to zero does not make much sense.

Ready after it finishes reg&bootstrap on x86_64-linux?

Thanks,
Martin

[-- Attachment #2: 0001-Fix-PR-rtl-optimization-71634.patch --]
[-- Type: text/x-patch, Size: 790 bytes --]

From e72dafdf3a2a7cfaca4a617fd10e80dd7aae1e91 Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
Date: Thu, 23 Jun 2016 12:52:44 +0200
Subject: [PATCH] Fix PR rtl-optimization/71634

gcc/ChangeLog:

2016-06-23  Martin Liska  <mliska@suse.cz>

	* params.def: Change min of ira-max-loops-num to 1.
---
 gcc/params.def | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/params.def b/gcc/params.def
index 894b7f3..1273cc9 100644
--- a/gcc/params.def
+++ b/gcc/params.def
@@ -810,7 +810,7 @@ DEFPARAM (PARAM_SCCVN_MAX_ALIAS_QUERIES_PER_ACCESS,
 DEFPARAM (PARAM_IRA_MAX_LOOPS_NUM,
 	  "ira-max-loops-num",
 	  "Max loops number for regional RA.",
-	  100, 0, 0)
+	  100, 1, 0)
 
 DEFPARAM (PARAM_IRA_MAX_CONFLICT_TABLE_SIZE,
 	  "ira-max-conflict-table-size",
-- 
2.8.4

Re: [PATCH] Fix PR rtl-optimization/71634

[Martin Liška]

PING^1

On 06/23/2016 12:56 PM, Martin Liška wrote:
> Hello.
> 
> Following patch changes minimum of ira-max-loops-num to 1.
> Having the minimum equal to zero does not make much sense.
> 
> Ready after it finishes reg&bootstrap on x86_64-linux?
> 
> Thanks,
> Martin
> 

Re: [PATCH] Fix PR rtl-optimization/71634

[Bernd Schmidt]

On 06/23/2016 12:56 PM, Martin Liška wrote:
> Following patch changes minimum of ira-max-loops-num to 1.
> Having the minimum equal to zero does not make much sense.
>
> Ready after it finishes reg&bootstrap on x86_64-linux?

Hmm, why wouldn't a number of zero make sense if you want try to have 
all loops removed?

The problem seems to be here:

   for (i = 0; n - i + 1 > IRA_MAX_LOOPS_NUM; i++)
     {
       sorted_loops[i]->to_remove_p = true;

and this looks like an off-by-one error. n is the number of elements in 
the array, so if IRA_MAX_LOOPS_NUM is 1, it'll iterate i from 0 up to 
n-1, where
   n - i + 1 == 2 > 1
So it'll clear everything, where it seems like it should leave one loop 
around.

So maybe write this with a standard form of for loop so it's actually 
comprehensible:

   int maxidx = MIN (IRA_MAX_LOOPS_NUM, n);
   for (i = 0; i < maxidx; i++)
     {
        ....


Bernd

Re: [PATCH] Fix PR rtl-optimization/71634

[Bernd Schmidt]

On 07/08/2016 01:52 PM, Bernd Schmidt wrote:
>   int maxidx = MIN (IRA_MAX_LOOPS_NUM, n);
>   for (i = 0; i < maxidx; i++)
>     {

Gah, that's not right, that'll swap the numbers of kept/removed loops.

I think the right answer is simply
   for (i = 0; i < n - IRA_MAX_LOOPS_NUM; i++)


Bernd

Re: [PATCH] Fix PR rtl-optimization/71634

[Martin Liška]

On 07/08/2016 01:59 PM, Bernd Schmidt wrote:
> 
> Gah, that's not right, that'll swap the numbers of kept/removed loops.
> 
> I think the right answer is simply
>   for (i = 0; i < n - IRA_MAX_LOOPS_NUM; i++)
> 
> 
> Bernd

Thank you for the help, I've been testing the suggested change.

Martin

Re: [PATCH] Fix PR rtl-optimization/71634

[Martin Liška]

On 07/08/2016 02:54 PM, Martin Liška wrote:
> On 07/08/2016 01:59 PM, Bernd Schmidt wrote:
>>
>> Gah, that's not right, that'll swap the numbers of kept/removed loops.
>>
>> I think the right answer is simply
>>   for (i = 0; i < n - IRA_MAX_LOOPS_NUM; i++)
>>
>>
>> Bernd
> 
> Thank you for the help, I've been testing the suggested change.
> 
> Martin
> 

It survives regression tests and bootstrap.
May I install the patch?

Thanks,
Martin

Re: [PATCH] Fix PR rtl-optimization/71634

[Bernd Schmidt]

On 07/08/2016 04:27 PM, Martin Liška wrote:
> On 07/08/2016 02:54 PM, Martin Liška wrote:
>> On 07/08/2016 01:59 PM, Bernd Schmidt wrote:
>>>
>>> Gah, that's not right, that'll swap the numbers of kept/removed loops.
>>>
>>> I think the right answer is simply
>>>   for (i = 0; i < n - IRA_MAX_LOOPS_NUM; i++)
>>>
>>>
>>> Bernd
>>
>> Thank you for the help, I've been testing the suggested change.
>>
>> Martin
>>
>
> It survives regression tests and bootstrap.
> May I install the patch?

Sure.


Bernd

Re: [PATCH] Fix PR rtl-optimization/71634

[Martin Liška]

On 07/12/2016 04:18 PM, Bernd Schmidt wrote:
> 
> 
> On 07/08/2016 04:27 PM, Martin Liška wrote:
>> On 07/08/2016 02:54 PM, Martin Liška wrote:
>>> On 07/08/2016 01:59 PM, Bernd Schmidt wrote:
>>>>
>>>> Gah, that's not right, that'll swap the numbers of kept/removed loops.
>>>>
>>>> I think the right answer is simply
>>>>   for (i = 0; i < n - IRA_MAX_LOOPS_NUM; i++)
>>>>
>>>>
>>>> Bernd
>>>
>>> Thank you for the help, I've been testing the suggested change.
>>>
>>> Martin
>>>
>>
>> It survives regression tests and bootstrap.
>> May I install the patch?
> 
> Sure.
> 
> 
> Bernd
> 

All active branches are also affected, hope it's fine that I'll commit the same
patch as soon as regression tests and bootstrap finishes?

Thanks,
Martin