| Message ID | 08a87e0d-4091-03c9-db2c-cf8686525d10@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | Fix typos in move_sese_region_to_fn | expand |
On Fri, Jul 30, 2021 at 1:15 PM Kewen.Lin <linkw@linux.ibm.com> wrote: > > Hi, > > This patch is to fix the typos in the move_sese_region_to_fn. > As mentioned here [1], I tried to debug the test case > gcc.dg/graphite/pr83359.c with trunk, but I found it didn't > go into the hunk guard with "if (moved_orig_loop_num)". So > I switched to commit 555758de90074 (also reproduced the ICE > with 555758de90074~ to ensure my used command step is correct), > I noticed the compilation of the test case only covers the > hunk > > else > { > moved_orig_loop_num[dloop->orig_loop_num] = -1; > dloop->orig_loop_num = 0; > } > > it doesn't touch the hunk > > if ((*larray)[dloop->orig_loop_num] != NULL > && get_loop (saved_cfun, dloop->orig_loop_num) == NULL) > { > if (moved_orig_loop_num[dloop->orig_loop_num] >= 0 > && moved_orig_loop_num[dloop->orig_loop_num] < 2) > moved_orig_loop_num[dloop->orig_loop_num]++; > dloop->orig_loop_num = (*larray)[dloop->orig_loop_num]->num; > } > > so the following hunk using dloop and guarded with > "if (moved_orig_loop_num[orig_loop_num] == 2)" doesn't get executed. > > It explains why the problem doesn't get exposed before. > > By looking to the code using dloop, I think it's a copy/paste typo, > the assertion > > gcc_assert ((*larray)[dloop->orig_loop_num] != NULL > && (get_loop (saved_cfun, dloop->orig_loop_num) > == NULL)); > > would like to ensure the condition in the previous > loop iterating is true, that is: > > if ((*larray)[dloop->orig_loop_num] != NULL > && get_loop (saved_cfun, dloop->orig_loop_num) == NULL) > > But in that context, I think the expected original number has been > assigned to variable orig_loop_num by extracting from the arg0 > of IFN_LOOP_DIST_ALIAS call. So replace those ones. > > Is it ok for trunk? OK. Thanks, Richard. > [1] https://gcc.gnu.org/pipermail/gcc-patches/2021-July/576367.html > > BR, > Kewen > ----- > gcc/ChangeLog: > > * tree-cfg.c (move_sese_region_to_fn): Fix typos on dloop.
diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c index 48ee8c011ab..9883eaaa9bf 100644 --- a/gcc/tree-cfg.c +++ b/gcc/tree-cfg.c @@ -7747,9 +7747,8 @@ move_sese_region_to_fn (struct function *dest_cfun, basic_block entry_bb, /* Fix up orig_loop_num. If the block referenced in it has been moved to dest_cfun, update orig_loop_num field, otherwise clear it. */ - class loop *dloop = NULL; signed char *moved_orig_loop_num = NULL; - for (class loop *dloop : loops_list (dest_cfun, 0)) + for (auto dloop : loops_list (dest_cfun, 0)) if (dloop->orig_loop_num) { if (moved_orig_loop_num == NULL) @@ -7787,11 +7786,10 @@ move_sese_region_to_fn (struct function *dest_cfun, basic_block entry_bb, /* If we have moved both loops with this orig_loop_num into dest_cfun and the LOOP_DIST_ALIAS call is being moved there too, update the first argument. */ - gcc_assert ((*larray)[dloop->orig_loop_num] != NULL - && (get_loop (saved_cfun, dloop->orig_loop_num) - == NULL)); + gcc_assert ((*larray)[orig_loop_num] != NULL + && (get_loop (saved_cfun, orig_loop_num) == NULL)); tree t = build_int_cst (integer_type_node, - (*larray)[dloop->orig_loop_num]->num); + (*larray)[orig_loop_num]->num); gimple_call_set_arg (g, 0, t); update_stmt (g); /* Make sure the following loop will not update it. */
Hi, This patch is to fix the typos in the move_sese_region_to_fn. As mentioned here [1], I tried to debug the test case gcc.dg/graphite/pr83359.c with trunk, but I found it didn't go into the hunk guard with "if (moved_orig_loop_num)". So I switched to commit 555758de90074 (also reproduced the ICE with 555758de90074~ to ensure my used command step is correct), I noticed the compilation of the test case only covers the hunk else { moved_orig_loop_num[dloop->orig_loop_num] = -1; dloop->orig_loop_num = 0; } it doesn't touch the hunk if ((*larray)[dloop->orig_loop_num] != NULL && get_loop (saved_cfun, dloop->orig_loop_num) == NULL) { if (moved_orig_loop_num[dloop->orig_loop_num] >= 0 && moved_orig_loop_num[dloop->orig_loop_num] < 2) moved_orig_loop_num[dloop->orig_loop_num]++; dloop->orig_loop_num = (*larray)[dloop->orig_loop_num]->num; } so the following hunk using dloop and guarded with "if (moved_orig_loop_num[orig_loop_num] == 2)" doesn't get executed. It explains why the problem doesn't get exposed before. By looking to the code using dloop, I think it's a copy/paste typo, the assertion gcc_assert ((*larray)[dloop->orig_loop_num] != NULL && (get_loop (saved_cfun, dloop->orig_loop_num) == NULL)); would like to ensure the condition in the previous loop iterating is true, that is: if ((*larray)[dloop->orig_loop_num] != NULL && get_loop (saved_cfun, dloop->orig_loop_num) == NULL) But in that context, I think the expected original number has been assigned to variable orig_loop_num by extracting from the arg0 of IFN_LOOP_DIST_ALIAS call. So replace those ones. Is it ok for trunk? [1] https://gcc.gnu.org/pipermail/gcc-patches/2021-July/576367.html BR, Kewen ----- gcc/ChangeLog: * tree-cfg.c (move_sese_region_to_fn): Fix typos on dloop.