| Message ID | 075b8346-d1fa-719a-4a6f-6d0391c15e9c@acm.org |
|---|---|
| State | New |
| Headers | show |
On Tue, May 2, 2017 at 3:41 PM, Nathan Sidwell <nathan@acm.org> wrote: > This loop in ggc-page confused me, because the iterator is one greater than > the indexing value. Also the formatting of the array indexing is incorrect. > > Fixed thusly, and applied as obvious after booting on x86_64-linux-gnu - for (i = G.by_depth_in_use; i > 0; --i) + for (unsigned i = G.by_depth_in_use; i--;) { - page_entry *p = G.by_depth[i-1]; - p->index_by_depth = i-1; + page_entry *p = G.by_depth[i]; + p->index_by_depth = i; } I think this is still incorrect. you replaced i - 1 by i but did not start at G.by_depth_in_use - 1; Thanks, Andrew > > nathan > -- > Nathan Sidwell
On Wed, May 3, 2017 at 2:09 AM, Andrew Pinski <pinskia@gmail.com> wrote: > On Tue, May 2, 2017 at 3:41 PM, Nathan Sidwell <nathan@acm.org> wrote: >> This loop in ggc-page confused me, because the iterator is one greater than >> the indexing value. Also the formatting of the array indexing is incorrect. >> >> Fixed thusly, and applied as obvious after booting on x86_64-linux-gnu > > - for (i = G.by_depth_in_use; i > 0; --i) > + for (unsigned i = G.by_depth_in_use; i--;) > { > - page_entry *p = G.by_depth[i-1]; > - p->index_by_depth = i-1; > + page_entry *p = G.by_depth[i]; > + p->index_by_depth = i; > } > > I think this is still incorrect. you replaced i - 1 by i but did not > start at G.by_depth_in_use - 1; He does, albeit in a coding way I dislike (watch how the iterator update is done in the condition!) Richard. > > Thanks, > Andrew > > >> >> nathan >> -- >> Nathan Sidwell
On 05/03/2017 05:54 AM, Richard Biener wrote: > He does, albeit in a coding way I dislike (watch how the iterator > update is done in the condition!) Sorry you dislike that. I've used that kind of loop idiom, since for ever though. nathan
Index: ggc-page.c =================================================================== --- ggc-page.c (revision 247528) +++ ggc-page.c (working copy) @@ -2507,8 +2507,6 @@ ggc_pch_finish (struct ggc_pch_data *d, static void move_ptes_to_front (int count_old_page_tables, int count_new_page_tables) { - unsigned i; - /* First, we swap the new entries to the front of the varrays. */ page_entry **new_by_depth; unsigned long **new_save_in_use; @@ -2536,10 +2534,10 @@ move_ptes_to_front (int count_old_page_t G.save_in_use = new_save_in_use; /* Now update all the index_by_depth fields. */ - for (i = G.by_depth_in_use; i > 0; --i) + for (unsigned i = G.by_depth_in_use; i--;) { - page_entry *p = G.by_depth[i-1]; - p->index_by_depth = i-1; + page_entry *p = G.by_depth[i]; + p->index_by_depth = i; } /* And last, we update the depth pointers in G.depth. The first