Re: [PATCH]ira: recompute regstat as max_regno changes [PR97705]
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
Re: [PATCH]ira: recompute regstat as max_regno changes [PR97705]
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.
[PATCH]ira: recompute regstat as max_regno changes [PR97705]
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. 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)