* [PATCH] Fix typos in move_sese_region_to_fn
@ 2021-07-30 11:15 Kewen.Lin
2021-08-02 9:49 ` Richard Biener
0 siblings, 1 reply; 2+ messages in thread
From: Kewen.Lin @ 2021-07-30 11:15 UTC (permalink / raw)
To: GCC Patches; +Cc: Richard Biener, Jakub Jelinek, Thomas Schwinge
[-- Attachment #1: Type: text/plain, Size: 1854 bytes --]
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.
[-- Attachment #2: move_sese_region_to_fn_typos.diff --]
[-- Type: text/plain, Size: 1408 bytes --]
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. */
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH] Fix typos in move_sese_region_to_fn
2021-07-30 11:15 [PATCH] Fix typos in move_sese_region_to_fn Kewen.Lin
@ 2021-08-02 9:49 ` Richard Biener
0 siblings, 0 replies; 2+ messages in thread
From: Richard Biener @ 2021-08-02 9:49 UTC (permalink / raw)
To: Kewen.Lin; +Cc: GCC Patches, Jakub Jelinek, Thomas Schwinge
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.
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2021-08-02 9:49 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-30 11:15 [PATCH] Fix typos in move_sese_region_to_fn Kewen.Lin
2021-08-02 9:49 ` Richard Biener
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).