[PATCH]ira: recompute regstat as max_regno changes [PR97705]

[PATCH]ira: recompute regstat as max_regno changes [PR97705]

[Kewen.Lin]

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

Hi,

As PR97705 shows, my commit r11-4637 caused some dumping
comparison difference error on pass ira.  It exposed one
issue about the newly introduced function remove_scratches,
which can increase the largest pseudo reg number if it
succeeds, later some function will use the max_reg_num()
to get the latest max_regno, when iterating the numbers
we can access some data structures which are allocated as
the previous max_regno, some out of array bound accesses
can occur, the failure can be random since the values
beyond the array could be random.

This patch is to free/reinit/recompute the relevant data
structures that is regstat_n_sets_and_refs and reg_info_p
to ensure we won't access beyond some array bounds.

Bootstrapped/regtested on powerpc64le-linux-gnu P9 and
powerpc64-linux-gnu P8.

Any thoughts?  Is it a reasonable fix?

BR,
Kewen
-----
gcc/ChangeLog:

	PR rtl-optimization/97705
	* ira.c (ira): Refactor some regstat free/init/compute invocation
	into lambda function regstat_recompute_for_max_regno, and call it
	when max_regno increases as remove_scratches succeeds.

[-- Attachment #2: ira_recomp.diff --]
[-- Type: text/plain, Size: 1470 bytes --]

diff --git a/gcc/ira.c b/gcc/ira.c
index 050405f1833..5443031674e 100644
--- a/gcc/ira.c
+++ b/gcc/ira.c
@@ -5526,8 +5526,26 @@ ira (FILE *f)
   end_alias_analysis ();
   free (reg_equiv);
 
+  /* Once max_regno changes, we need to free and re-init/re-compute
+     some data structures like regstat_n_sets_and_refs and reg_info_p.  */
+  auto regstat_recompute_for_max_regno = []() {
+    regstat_free_n_sets_and_refs ();
+    regstat_free_ri ();
+    regstat_init_n_sets_and_refs ();
+    regstat_compute_ri ();
+  };
+
+  int max_regno_before_rm = max_reg_num ();
   if (ira_use_lra_p && remove_scratches ())
-    ira_expand_reg_equiv ();
+    {
+      ira_expand_reg_equiv ();
+      /* For now remove_scatches is supposed to create pseudos when it
+	 succeeds, assert this happens all the time.  Once it doesn't
+	 hold, we should guard the regstat recompute for the case
+	 max_regno changes.  */
+      gcc_assert (max_regno_before_rm != max_reg_num ());
+      regstat_recompute_for_max_regno ();
+    }
 
   if (resize_reg_info () && flag_ira_loop_pressure)
     ira_set_pseudo_classes (true, ira_dump_file);
@@ -5654,12 +5672,7 @@ ira (FILE *f)
 #endif
 
   if (max_regno != max_regno_before_ira)
-    {
-      regstat_free_n_sets_and_refs ();
-      regstat_free_ri ();
-      regstat_init_n_sets_and_refs ();
-      regstat_compute_ri ();
-    }
+    regstat_recompute_for_max_regno ();
 
   overall_cost_before = ira_overall_cost;
   if (! ira_conflicts_p)

Re: [PATCH]ira: recompute regstat as max_regno changes [PR97705]

[Vladimir Makarov]

On 2020-11-06 1:15 a.m., Kewen.Lin wrote:
> Hi,
>
> As PR97705 shows, my commit r11-4637 caused some dumping
> comparison difference error on pass ira.  It exposed one
> issue about the newly introduced function remove_scratches,
> which can increase the largest pseudo reg number if it
> succeeds, later some function will use the max_reg_num()
> to get the latest max_regno, when iterating the numbers
> we can access some data structures which are allocated as
> the previous max_regno, some out of array bound accesses
> can occur, the failure can be random since the values
> beyond the array could be random.
>
> This patch is to free/reinit/recompute the relevant data
> structures that is regstat_n_sets_and_refs and reg_info_p
> to ensure we won't access beyond some array bounds.
>
> Bootstrapped/regtested on powerpc64le-linux-gnu P9 and
> powerpc64-linux-gnu P8.
>
> Any thoughts?  Is it a reasonable fix?
>
Sure, Kewen.  A bit unexpected to see lambda to use for this but I 
checked and found couple places in GCC where lambdas are already used.

The patch is ok.  Please, commit it to the mainline.

Thank you for the patch.

Re: [PATCH]ira: recompute regstat as max_regno changes [PR97705]

[Kewen.Lin]

Hi Vladimir,

on 2020/11/6 下午10:49, Vladimir Makarov wrote:
> 
> On 2020-11-06 1:15 a.m., Kewen.Lin wrote:
>> Hi,
>>
>> As PR97705 shows, my commit r11-4637 caused some dumping
>> comparison difference error on pass ira.  It exposed one
>> issue about the newly introduced function remove_scratches,
>> which can increase the largest pseudo reg number if it
>> succeeds, later some function will use the max_reg_num()
>> to get the latest max_regno, when iterating the numbers
>> we can access some data structures which are allocated as
>> the previous max_regno, some out of array bound accesses
>> can occur, the failure can be random since the values
>> beyond the array could be random.
>>
>> This patch is to free/reinit/recompute the relevant data
>> structures that is regstat_n_sets_and_refs and reg_info_p
>> to ensure we won't access beyond some array bounds.
>>
>> Bootstrapped/regtested on powerpc64le-linux-gnu P9 and
>> powerpc64-linux-gnu P8.
>>
>> Any thoughts?  Is it a reasonable fix?
>>
> Sure, Kewen.  A bit unexpected to see lambda to use for this but I checked and found couple places in GCC where lambdas are already used.

Thanks for your prompt review!

Yeah, gcc11 build requires C++11 support as noted in changes.html.

> 
> The patch is ok.  Please, commit it to the mainline.

Thanks.  Committed in r11-4827.

BR,
Kewen