Re: [PATCH v2] gcov: Fix integer types in gen_counter_update()
On 23.11.23 18:29, Sebastian Huber wrote:
On 23.11.23 09:20, Sebastian Huber wrote:
On 23.11.23 09:11, Jiang, Haochen wrote:
-Original Message-
From: Sebastian Huber
Sent: Wednesday, November 22, 2023 10:24 PM
To: Christophe Lyon
Cc: Jakub Jelinek;gcc-patches@gcc.gnu.org
Subject: Re: [PATCH v2] gcov: Fix integer types in gen_counter_update()
On 22.11.23 15:22, Christophe Lyon wrote:
On Tue, 21 Nov 2023 at 12:22, Sebastian Huber
wrote:
On 21.11.23 11:46, Jakub Jelinek wrote:
On Tue, Nov 21, 2023 at 11:42:06AM +0100, Sebastian Huber wrote:
On 21.11.23 11:34, Jakub Jelinek wrote:
--- a/gcc/tree-profile.cc
+++ b/gcc/tree-profile.cc
@@ -281,10 +281,13 @@ gen_assign_counter_update
(gimple_stmt_iterator *gsi, gcall *call, tree func,
if (result)
{
tree result_type = TREE_TYPE (TREE_TYPE (func));
- tree tmp = make_temp_ssa_name (result_type, NULL, name);
- gimple_set_lhs (call, tmp);
+ tree tmp1 = make_temp_ssa_name (result_type, NULL, name);
+ gimple_set_lhs (call, tmp1);
gsi_insert_after (gsi, call, GSI_NEW_STMT);
- gassign *assign = gimple_build_assign (result, tmp);
+ tree tmp2 = make_ssa_name (TREE_TYPE (result));
+ gassign *assign = gimple_build_assign (tmp2, NOP_EXPR,
tmp1);
+ gsi_insert_after (gsi, assign, GSI_NEW_STMT);
+ assign = gimple_build_assign (result, gimple_assign_lhs
(assign));
When you use a temporary tmp2 for the lhs of the conversion,
you can
just
use it here,
assign = gimple_build_assign (result, tmp2);
Ok for trunk with that change.
Just a question, could I also use
tree tmp2 = make_temp_ssa_name (TREE_TYPE (result), NULL, name);
?
This make_temp_ssa_name() is used throughout the file and the new
make_ssa_name() would be the first use in this file.
Yes. The only difference is that it won't be _234 = (type)
something;
but PROF_time_profile_234 = (type) something; in the dumps, but
sure,
consistency is useful.
Thanks for your help. I checked in an updated version.
Our CI bisected a regression to this commit:
Running gcc:gcc.dg/tree-prof/tree-prof.exp ...
FAIL: gcc.dg/tree-prof/time-profiler-3.c scan-ipa-dump-times profile
"Read tp_first_run: 0" 1
FAIL: gcc.dg/tree-prof/time-profiler-3.c scan-ipa-dump-times profile
"Read tp_first_run: 2" 1
(on aarch64)
Can you check?
Yes, I will have a look at it.
The same issue also happened on i386. You can also reproduce that on
x86-64 platforms.
I was able to reproduce it using a native aarch64 GCC on cfarm185, but
I have some difficulties to understand what this test case does actually.
Could the problem be caused by some other recent commit?
I built this commit:
commit 3ef8882adcb1ab5fa535e9e1a2a28ea7c8eeca4f
Author: Sebastian Huber
Date: Tue Nov 14 21:27:37 2023 +0100
Add TARGET_HAVE_LIBATOMIC
Here, the test passes. If I build
commit e9b39df9333d565ee06fa65d62bfca4bbcb92e44 (origin/trunk,
origin/master, origin/HEAD)
Author: Iain Sandoe
Date: Tue Nov 21 10:19:29 2023 +
testsuite: Update path to intl include.
the test fails.
To check that my changes caused the problem, I applied the following
patches to 3ef8882adcb1ab5fa535e9e1a2a28ea7c8eeca4f (git cherry-pick):
Author: Sebastian Huber
Date: Sat Oct 21 15:52:15 2023 +0200
gcov: Add gen_counter_update()
Author: Sebastian Huber
Date: Mon Nov 20 14:48:03 2023 +0100
gcov: Use unshare_expr() in gen_counter_update()
Author: Sebastian Huber
Date: Mon Nov 20 15:26:38 2023 +0100
gcov: Fix integer types in gen_counter_update()
Author: Sebastian Huber
Date: Tue Nov 14 21:36:51 2023 +0100
gcov: Improve -fprofile-update=atomic
Author: Jakub Jelinek
Date: Tue Nov 21 10:49:51 2023 +0100
gcov: Formatting fixes
Author: Sebastian Huber
Date: Thu Nov 23 06:44:15 2023 +
gcov: Do not use atomic ops for -fprofile-update=single
With these changes alone on top of
3ef8882adcb1ab5fa535e9e1a2a28ea7c8eeca4f I don't see the regressions:
Executing on host: /home/sh/b-gcc/gcc/xgcc -B/home/sh/b-gcc/gcc/
/home/sh/gcc/gcc/testsuite/gcc.dg/tree-prof/time-profiler-3.c
-fdiagnostics-plain-output -O2 -fdump-ipa-profile
-fprofile-update=atomic -fprofile-generate -D_PROFILE_GENERATE
-dumpbase-ext .x01 -lm -o
/home/sh/b-gcc/gcc/testsuite/gcc5/time-profiler-3.x01 (timeout = 300)
spawn -ignore SIGHUP /home/sh/b-gcc/gcc/xgcc -B/home/sh/b-gcc/gcc/
/home/sh/gcc/gcc/testsuite/gcc.dg/tree-prof/time-profiler-3.c
-fdiagnostics-plain-output -O2 -fdump-ipa-profile
-fprofile-update=atomic -fprofile-generate -D_PROFILE_GENERATE
-dumpbase-ext .x01 -lm -o
/home/sh/b-gcc/gcc/testsuite/gcc5/time-profiler-3.x01
PASS: gcc.dg/tree-prof/time-profiler-3.c compilation, -fprofile-generate
-D_PROFILE_GENERATE
Setting LD_LIBRARY_PATH to
:/home/sh/b-gcc/gcc:/home/sh/b-gcc/aarch64-unknown-linux-gnu/./libatomic/.libs::/home/sh/b-gcc/gcc:/home/sh/b-gcc/aarch64-unknown-linux-gnu/./libatomic/.libs:/h
Re: [PATCH v2] gcov: Fix integer types in gen_counter_update()
On 23.11.23 09:20, Sebastian Huber wrote:
On 23.11.23 09:11, Jiang, Haochen wrote:
-Original Message-
From: Sebastian Huber
Sent: Wednesday, November 22, 2023 10:24 PM
To: Christophe Lyon
Cc: Jakub Jelinek;gcc-patches@gcc.gnu.org
Subject: Re: [PATCH v2] gcov: Fix integer types in gen_counter_update()
On 22.11.23 15:22, Christophe Lyon wrote:
On Tue, 21 Nov 2023 at 12:22, Sebastian Huber
wrote:
On 21.11.23 11:46, Jakub Jelinek wrote:
On Tue, Nov 21, 2023 at 11:42:06AM +0100, Sebastian Huber wrote:
On 21.11.23 11:34, Jakub Jelinek wrote:
--- a/gcc/tree-profile.cc
+++ b/gcc/tree-profile.cc
@@ -281,10 +281,13 @@ gen_assign_counter_update
(gimple_stmt_iterator *gsi, gcall *call, tree func,
if (result)
{
tree result_type = TREE_TYPE (TREE_TYPE (func));
- tree tmp = make_temp_ssa_name (result_type, NULL, name);
- gimple_set_lhs (call, tmp);
+ tree tmp1 = make_temp_ssa_name (result_type, NULL, name);
+ gimple_set_lhs (call, tmp1);
gsi_insert_after (gsi, call, GSI_NEW_STMT);
- gassign *assign = gimple_build_assign (result, tmp);
+ tree tmp2 = make_ssa_name (TREE_TYPE (result));
+ gassign *assign = gimple_build_assign (tmp2, NOP_EXPR,
tmp1);
+ gsi_insert_after (gsi, assign, GSI_NEW_STMT);
+ assign = gimple_build_assign (result, gimple_assign_lhs
(assign));
When you use a temporary tmp2 for the lhs of the conversion, you
can
just
use it here,
assign = gimple_build_assign (result, tmp2);
Ok for trunk with that change.
Just a question, could I also use
tree tmp2 = make_temp_ssa_name (TREE_TYPE (result), NULL, name);
?
This make_temp_ssa_name() is used throughout the file and the new
make_ssa_name() would be the first use in this file.
Yes. The only difference is that it won't be _234 = (type)
something;
but PROF_time_profile_234 = (type) something; in the dumps, but sure,
consistency is useful.
Thanks for your help. I checked in an updated version.
Our CI bisected a regression to this commit:
Running gcc:gcc.dg/tree-prof/tree-prof.exp ...
FAIL: gcc.dg/tree-prof/time-profiler-3.c scan-ipa-dump-times profile
"Read tp_first_run: 0" 1
FAIL: gcc.dg/tree-prof/time-profiler-3.c scan-ipa-dump-times profile
"Read tp_first_run: 2" 1
(on aarch64)
Can you check?
Yes, I will have a look at it.
The same issue also happened on i386. You can also reproduce that on
x86-64 platforms.
I was able to reproduce it using a native aarch64 GCC on cfarm185, but I
have some difficulties to understand what this test case does actually.
Could the problem be caused by some other recent commit?
I built this commit:
commit 3ef8882adcb1ab5fa535e9e1a2a28ea7c8eeca4f
Author: Sebastian Huber
Date: Tue Nov 14 21:27:37 2023 +0100
Add TARGET_HAVE_LIBATOMIC
Here, the test passes. If I build
commit e9b39df9333d565ee06fa65d62bfca4bbcb92e44 (origin/trunk,
origin/master, origin/HEAD)
Author: Iain Sandoe
Date: Tue Nov 21 10:19:29 2023 +
testsuite: Update path to intl include.
the test fails.
To check that my changes caused the problem, I applied the following
patches to 3ef8882adcb1ab5fa535e9e1a2a28ea7c8eeca4f (git cherry-pick):
Author: Sebastian Huber
Date: Sat Oct 21 15:52:15 2023 +0200
gcov: Add gen_counter_update()
Author: Sebastian Huber
Date: Mon Nov 20 14:48:03 2023 +0100
gcov: Use unshare_expr() in gen_counter_update()
Author: Sebastian Huber
Date: Mon Nov 20 15:26:38 2023 +0100
gcov: Fix integer types in gen_counter_update()
Author: Sebastian Huber
Date: Tue Nov 14 21:36:51 2023 +0100
gcov: Improve -fprofile-update=atomic
Author: Jakub Jelinek
Date: Tue Nov 21 10:49:51 2023 +0100
gcov: Formatting fixes
Author: Sebastian Huber
Date: Thu Nov 23 06:44:15 2023 +
gcov: Do not use atomic ops for -fprofile-update=single
With these changes alone on top of
3ef8882adcb1ab5fa535e9e1a2a28ea7c8eeca4f I don't see the regressions:
Executing on host: /home/sh/b-gcc/gcc/xgcc -B/home/sh/b-gcc/gcc/
/home/sh/gcc/gcc/testsuite/gcc.dg/tree-prof/time-profiler-3.c
-fdiagnostics-plain-output -O2 -fdump-ipa-profile
-fprofile-update=atomic -fprofile-generate -D_PROFILE_GENERATE
-dumpbase-ext .x01 -lm -o
/home/sh/b-gcc/gcc/testsuite/gcc5/time-profiler-3.x01(timeout = 300)
spawn -ignore SIGHUP /home/sh/b-gcc/gcc/xgcc -B/home/sh/b-gcc/gcc/
/home/sh/gcc/gcc/testsuite/gcc.dg/tree-prof/time-profiler-3.c
-fdiagnostics-plain-output -O2 -fdump-ipa-profile
-fprofile-update=atomic -fprofile-generate -D_PROFILE_GENERATE
-dumpbase-ext .x01 -lm -o
/home/sh/b-gcc/gcc/testsuite/gcc5/time-profiler-3.x01
PASS: gcc.dg/tree-prof/time-profiler-3.c compilation,
-fprofile-generate -D_PROFILE_GENERATE
Setting LD_LIBRARY_PATH to
:/home/sh/b-gcc/gcc:/home/sh/b-gcc/aarch64-unknown-linux-gnu/./libatomic/.libs::/home/sh/b-gcc/gcc:/home/sh/b-gcc/aarch64-unknown-linux-gnu/./libatomic/.libs:/home/sh/b-gcc/aarch64-unknown-linux-gnu/libstdc++-v3/
Re: [PATCH v2] gcov: Fix integer types in gen_counter_update()
On 23.11.23 09:11, Jiang, Haochen wrote:
-Original Message-
From: Sebastian Huber
Sent: Wednesday, November 22, 2023 10:24 PM
To: Christophe Lyon
Cc: Jakub Jelinek;gcc-patches@gcc.gnu.org
Subject: Re: [PATCH v2] gcov: Fix integer types in gen_counter_update()
On 22.11.23 15:22, Christophe Lyon wrote:
On Tue, 21 Nov 2023 at 12:22, Sebastian Huber
wrote:
On 21.11.23 11:46, Jakub Jelinek wrote:
On Tue, Nov 21, 2023 at 11:42:06AM +0100, Sebastian Huber wrote:
On 21.11.23 11:34, Jakub Jelinek wrote:
--- a/gcc/tree-profile.cc
+++ b/gcc/tree-profile.cc
@@ -281,10 +281,13 @@ gen_assign_counter_update
(gimple_stmt_iterator *gsi, gcall *call, tree func,
if (result)
{
tree result_type = TREE_TYPE (TREE_TYPE (func));
- tree tmp = make_temp_ssa_name (result_type, NULL, name);
- gimple_set_lhs (call, tmp);
+ tree tmp1 = make_temp_ssa_name (result_type, NULL, name);
+ gimple_set_lhs (call, tmp1);
gsi_insert_after (gsi, call, GSI_NEW_STMT);
- gassign *assign = gimple_build_assign (result, tmp);
+ tree tmp2 = make_ssa_name (TREE_TYPE (result));
+ gassign *assign = gimple_build_assign (tmp2, NOP_EXPR, tmp1);
+ gsi_insert_after (gsi, assign, GSI_NEW_STMT);
+ assign = gimple_build_assign (result, gimple_assign_lhs (assign));
When you use a temporary tmp2 for the lhs of the conversion, you can
just
use it here,
assign = gimple_build_assign (result, tmp2);
Ok for trunk with that change.
Just a question, could I also use
tree tmp2 = make_temp_ssa_name (TREE_TYPE (result), NULL, name);
?
This make_temp_ssa_name() is used throughout the file and the new
make_ssa_name() would be the first use in this file.
Yes. The only difference is that it won't be _234 = (type) something;
but PROF_time_profile_234 = (type) something; in the dumps, but sure,
consistency is useful.
Thanks for your help. I checked in an updated version.
Our CI bisected a regression to this commit:
Running gcc:gcc.dg/tree-prof/tree-prof.exp ...
FAIL: gcc.dg/tree-prof/time-profiler-3.c scan-ipa-dump-times profile
"Read tp_first_run: 0" 1
FAIL: gcc.dg/tree-prof/time-profiler-3.c scan-ipa-dump-times profile
"Read tp_first_run: 2" 1
(on aarch64)
Can you check?
Yes, I will have a look at it.
The same issue also happened on i386. You can also reproduce that on
x86-64 platforms.
I was able to reproduce it using a native aarch64 GCC on cfarm185, but I
have some difficulties to understand what this test case does actually.
--
embedded brains GmbH
Herr Sebastian HUBER
Dornierstr. 4
82178 Puchheim
Germany
email: sebastian.hu...@embedded-brains.de
phone: +49-89-18 94 741 - 16
fax: +49-89-18 94 741 - 08
Registergericht: Amtsgericht München
Registernummer: HRB 157899
Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
Unsere Datenschutzerklärung finden Sie hier:
https://embedded-brains.de/datenschutzerklaerung/
RE: [PATCH v2] gcov: Fix integer types in gen_counter_update()
> -Original Message-
> From: Sebastian Huber
> Sent: Wednesday, November 22, 2023 10:24 PM
> To: Christophe Lyon
> Cc: Jakub Jelinek ; gcc-patches@gcc.gnu.org
> Subject: Re: [PATCH v2] gcov: Fix integer types in gen_counter_update()
>
> On 22.11.23 15:22, Christophe Lyon wrote:
> > On Tue, 21 Nov 2023 at 12:22, Sebastian Huber
> > wrote:
> >> On 21.11.23 11:46, Jakub Jelinek wrote:
> >>> On Tue, Nov 21, 2023 at 11:42:06AM +0100, Sebastian Huber wrote:
> >>>> On 21.11.23 11:34, Jakub Jelinek wrote:
> >>>>>> --- a/gcc/tree-profile.cc
> >>>>>> +++ b/gcc/tree-profile.cc
> >>>>>> @@ -281,10 +281,13 @@ gen_assign_counter_update
> (gimple_stmt_iterator *gsi, gcall *call, tree func,
> >>>>>> if (result)
> >>>>>> {
> >>>>>> tree result_type = TREE_TYPE (TREE_TYPE (func));
> >>>>>> - tree tmp = make_temp_ssa_name (result_type, NULL, name);
> >>>>>> - gimple_set_lhs (call, tmp);
> >>>>>> + tree tmp1 = make_temp_ssa_name (result_type, NULL, name);
> >>>>>> + gimple_set_lhs (call, tmp1);
> >>>>>> gsi_insert_after (gsi, call, GSI_NEW_STMT);
> >>>>>> - gassign *assign = gimple_build_assign (result, tmp);
> >>>>>> + tree tmp2 = make_ssa_name (TREE_TYPE (result));
> >>>>>> + gassign *assign = gimple_build_assign (tmp2, NOP_EXPR, tmp1);
> >>>>>> + gsi_insert_after (gsi, assign, GSI_NEW_STMT);
> >>>>>> + assign = gimple_build_assign (result, gimple_assign_lhs
> >>>>>> (assign));
> >>>>> When you use a temporary tmp2 for the lhs of the conversion, you can
> just
> >>>>> use it here,
> >>>>> assign = gimple_build_assign (result, tmp2);
> >>>>>
> >>>>> Ok for trunk with that change.
> >>>> Just a question, could I also use
> >>>>
> >>>> tree tmp2 = make_temp_ssa_name (TREE_TYPE (result), NULL, name);
> >>>>
> >>>> ?
> >>>>
> >>>> This make_temp_ssa_name() is used throughout the file and the new
> >>>> make_ssa_name() would be the first use in this file.
> >>> Yes. The only difference is that it won't be _234 = (type) something;
> >>> but PROF_time_profile_234 = (type) something; in the dumps, but sure,
> >>> consistency is useful.
> >> Thanks for your help. I checked in an updated version.
> >>
> > Our CI bisected a regression to this commit:
> > Running gcc:gcc.dg/tree-prof/tree-prof.exp ...
> > FAIL: gcc.dg/tree-prof/time-profiler-3.c scan-ipa-dump-times profile
> > "Read tp_first_run: 0" 1
> > FAIL: gcc.dg/tree-prof/time-profiler-3.c scan-ipa-dump-times profile
> > "Read tp_first_run: 2" 1
> >
> > (on aarch64)
> >
> > Can you check?
>
> Yes, I will have a look at it.
The same issue also happened on i386. You can also reproduce that on
x86-64 platforms.
Thx,
Haochen
>
> --
> embedded brains GmbH
> Herr Sebastian HUBER
> Dornierstr. 4
> 82178 Puchheim
> Germany
> email: sebastian.hu...@embedded-brains.de
> phone: +49-89-18 94 741 - 16
> fax: +49-89-18 94 741 - 08
>
> Registergericht: Amtsgericht München
> Registernummer: HRB 157899
> Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
> Unsere Datenschutzerklärung finden Sie hier:
> https://embedded-brains.de/datenschutzerklaerung/
Re: [PATCH v2] gcov: Fix integer types in gen_counter_update()
On 22.11.23 15:22, Christophe Lyon wrote:
On Tue, 21 Nov 2023 at 12:22, Sebastian Huber
wrote:
On 21.11.23 11:46, Jakub Jelinek wrote:
On Tue, Nov 21, 2023 at 11:42:06AM +0100, Sebastian Huber wrote:
On 21.11.23 11:34, Jakub Jelinek wrote:
--- a/gcc/tree-profile.cc
+++ b/gcc/tree-profile.cc
@@ -281,10 +281,13 @@ gen_assign_counter_update (gimple_stmt_iterator *gsi,
gcall *call, tree func,
if (result)
{
tree result_type = TREE_TYPE (TREE_TYPE (func));
- tree tmp = make_temp_ssa_name (result_type, NULL, name);
- gimple_set_lhs (call, tmp);
+ tree tmp1 = make_temp_ssa_name (result_type, NULL, name);
+ gimple_set_lhs (call, tmp1);
gsi_insert_after (gsi, call, GSI_NEW_STMT);
- gassign *assign = gimple_build_assign (result, tmp);
+ tree tmp2 = make_ssa_name (TREE_TYPE (result));
+ gassign *assign = gimple_build_assign (tmp2, NOP_EXPR, tmp1);
+ gsi_insert_after (gsi, assign, GSI_NEW_STMT);
+ assign = gimple_build_assign (result, gimple_assign_lhs (assign));
When you use a temporary tmp2 for the lhs of the conversion, you can just
use it here,
assign = gimple_build_assign (result, tmp2);
Ok for trunk with that change.
Just a question, could I also use
tree tmp2 = make_temp_ssa_name (TREE_TYPE (result), NULL, name);
?
This make_temp_ssa_name() is used throughout the file and the new
make_ssa_name() would be the first use in this file.
Yes. The only difference is that it won't be _234 = (type) something;
but PROF_time_profile_234 = (type) something; in the dumps, but sure,
consistency is useful.
Thanks for your help. I checked in an updated version.
Our CI bisected a regression to this commit:
Running gcc:gcc.dg/tree-prof/tree-prof.exp ...
FAIL: gcc.dg/tree-prof/time-profiler-3.c scan-ipa-dump-times profile
"Read tp_first_run: 0" 1
FAIL: gcc.dg/tree-prof/time-profiler-3.c scan-ipa-dump-times profile
"Read tp_first_run: 2" 1
(on aarch64)
Can you check?
Yes, I will have a look at it.
--
embedded brains GmbH
Herr Sebastian HUBER
Dornierstr. 4
82178 Puchheim
Germany
email: sebastian.hu...@embedded-brains.de
phone: +49-89-18 94 741 - 16
fax: +49-89-18 94 741 - 08
Registergericht: Amtsgericht München
Registernummer: HRB 157899
Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
Unsere Datenschutzerklärung finden Sie hier:
https://embedded-brains.de/datenschutzerklaerung/
Re: [PATCH v2] gcov: Fix integer types in gen_counter_update()
Hi,
On Tue, 21 Nov 2023 at 12:22, Sebastian Huber
wrote:
>
> On 21.11.23 11:46, Jakub Jelinek wrote:
> > On Tue, Nov 21, 2023 at 11:42:06AM +0100, Sebastian Huber wrote:
> >>
> >> On 21.11.23 11:34, Jakub Jelinek wrote:
> --- a/gcc/tree-profile.cc
> +++ b/gcc/tree-profile.cc
> @@ -281,10 +281,13 @@ gen_assign_counter_update (gimple_stmt_iterator
> *gsi, gcall *call, tree func,
> if (result)
> {
> tree result_type = TREE_TYPE (TREE_TYPE (func));
> - tree tmp = make_temp_ssa_name (result_type, NULL, name);
> - gimple_set_lhs (call, tmp);
> + tree tmp1 = make_temp_ssa_name (result_type, NULL, name);
> + gimple_set_lhs (call, tmp1);
> gsi_insert_after (gsi, call, GSI_NEW_STMT);
> - gassign *assign = gimple_build_assign (result, tmp);
> + tree tmp2 = make_ssa_name (TREE_TYPE (result));
> + gassign *assign = gimple_build_assign (tmp2, NOP_EXPR, tmp1);
> + gsi_insert_after (gsi, assign, GSI_NEW_STMT);
> + assign = gimple_build_assign (result, gimple_assign_lhs (assign));
> >>> When you use a temporary tmp2 for the lhs of the conversion, you can just
> >>> use it here,
> >>> assign = gimple_build_assign (result, tmp2);
> >>>
> >>> Ok for trunk with that change.
> >> Just a question, could I also use
> >>
> >> tree tmp2 = make_temp_ssa_name (TREE_TYPE (result), NULL, name);
> >>
> >> ?
> >>
> >> This make_temp_ssa_name() is used throughout the file and the new
> >> make_ssa_name() would be the first use in this file.
> > Yes. The only difference is that it won't be _234 = (type) something;
> > but PROF_time_profile_234 = (type) something; in the dumps, but sure,
> > consistency is useful.
>
> Thanks for your help. I checked in an updated version.
>
Our CI bisected a regression to this commit:
Running gcc:gcc.dg/tree-prof/tree-prof.exp ...
FAIL: gcc.dg/tree-prof/time-profiler-3.c scan-ipa-dump-times profile
"Read tp_first_run: 0" 1
FAIL: gcc.dg/tree-prof/time-profiler-3.c scan-ipa-dump-times profile
"Read tp_first_run: 2" 1
(on aarch64)
Can you check?
Thanks,
Christophe
> --
> embedded brains GmbH
> Herr Sebastian HUBER
> Dornierstr. 4
> 82178 Puchheim
> Germany
> email: sebastian.hu...@embedded-brains.de
> phone: +49-89-18 94 741 - 16
> fax: +49-89-18 94 741 - 08
>
> Registergericht: Amtsgericht München
> Registernummer: HRB 157899
> Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
> Unsere Datenschutzerklärung finden Sie hier:
> https://embedded-brains.de/datenschutzerklaerung/
Re: [PATCH v2] gcov: Fix integer types in gen_counter_update()
On 21.11.23 11:46, Jakub Jelinek wrote:
On Tue, Nov 21, 2023 at 11:42:06AM +0100, Sebastian Huber wrote:
On 21.11.23 11:34, Jakub Jelinek wrote:
--- a/gcc/tree-profile.cc
+++ b/gcc/tree-profile.cc
@@ -281,10 +281,13 @@ gen_assign_counter_update (gimple_stmt_iterator *gsi,
gcall *call, tree func,
if (result)
{
tree result_type = TREE_TYPE (TREE_TYPE (func));
- tree tmp = make_temp_ssa_name (result_type, NULL, name);
- gimple_set_lhs (call, tmp);
+ tree tmp1 = make_temp_ssa_name (result_type, NULL, name);
+ gimple_set_lhs (call, tmp1);
gsi_insert_after (gsi, call, GSI_NEW_STMT);
- gassign *assign = gimple_build_assign (result, tmp);
+ tree tmp2 = make_ssa_name (TREE_TYPE (result));
+ gassign *assign = gimple_build_assign (tmp2, NOP_EXPR, tmp1);
+ gsi_insert_after (gsi, assign, GSI_NEW_STMT);
+ assign = gimple_build_assign (result, gimple_assign_lhs (assign));
When you use a temporary tmp2 for the lhs of the conversion, you can just
use it here,
assign = gimple_build_assign (result, tmp2);
Ok for trunk with that change.
Just a question, could I also use
tree tmp2 = make_temp_ssa_name (TREE_TYPE (result), NULL, name);
?
This make_temp_ssa_name() is used throughout the file and the new
make_ssa_name() would be the first use in this file.
Yes. The only difference is that it won't be _234 = (type) something;
but PROF_time_profile_234 = (type) something; in the dumps, but sure,
consistency is useful.
Thanks for your help. I checked in an updated version.
--
embedded brains GmbH
Herr Sebastian HUBER
Dornierstr. 4
82178 Puchheim
Germany
email: sebastian.hu...@embedded-brains.de
phone: +49-89-18 94 741 - 16
fax: +49-89-18 94 741 - 08
Registergericht: Amtsgericht München
Registernummer: HRB 157899
Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
Unsere Datenschutzerklärung finden Sie hier:
https://embedded-brains.de/datenschutzerklaerung/
Re: [PATCH v2] gcov: Fix integer types in gen_counter_update()
On Tue, Nov 21, 2023 at 11:42:06AM +0100, Sebastian Huber wrote:
>
>
> On 21.11.23 11:34, Jakub Jelinek wrote:
> > > --- a/gcc/tree-profile.cc
> > > +++ b/gcc/tree-profile.cc
> > > @@ -281,10 +281,13 @@ gen_assign_counter_update (gimple_stmt_iterator
> > > *gsi, gcall *call, tree func,
> > > if (result)
> > > {
> > > tree result_type = TREE_TYPE (TREE_TYPE (func));
> > > - tree tmp = make_temp_ssa_name (result_type, NULL, name);
> > > - gimple_set_lhs (call, tmp);
> > > + tree tmp1 = make_temp_ssa_name (result_type, NULL, name);
> > > + gimple_set_lhs (call, tmp1);
> > > gsi_insert_after (gsi, call, GSI_NEW_STMT);
> > > - gassign *assign = gimple_build_assign (result, tmp);
> > > + tree tmp2 = make_ssa_name (TREE_TYPE (result));
> > > + gassign *assign = gimple_build_assign (tmp2, NOP_EXPR, tmp1);
> > > + gsi_insert_after (gsi, assign, GSI_NEW_STMT);
> > > + assign = gimple_build_assign (result, gimple_assign_lhs (assign));
> > When you use a temporary tmp2 for the lhs of the conversion, you can just
> > use it here,
> >assign = gimple_build_assign (result, tmp2);
> >
> > Ok for trunk with that change.
>
> Just a question, could I also use
>
> tree tmp2 = make_temp_ssa_name (TREE_TYPE (result), NULL, name);
>
> ?
>
> This make_temp_ssa_name() is used throughout the file and the new
> make_ssa_name() would be the first use in this file.
Yes. The only difference is that it won't be _234 = (type) something;
but PROF_time_profile_234 = (type) something; in the dumps, but sure,
consistency is useful.
Jakub
Re: [PATCH v2] gcov: Fix integer types in gen_counter_update()
On 21.11.23 11:34, Jakub Jelinek wrote:
--- a/gcc/tree-profile.cc
+++ b/gcc/tree-profile.cc
@@ -281,10 +281,13 @@ gen_assign_counter_update (gimple_stmt_iterator *gsi,
gcall *call, tree func,
if (result)
{
tree result_type = TREE_TYPE (TREE_TYPE (func));
- tree tmp = make_temp_ssa_name (result_type, NULL, name);
- gimple_set_lhs (call, tmp);
+ tree tmp1 = make_temp_ssa_name (result_type, NULL, name);
+ gimple_set_lhs (call, tmp1);
gsi_insert_after (gsi, call, GSI_NEW_STMT);
- gassign *assign = gimple_build_assign (result, tmp);
+ tree tmp2 = make_ssa_name (TREE_TYPE (result));
+ gassign *assign = gimple_build_assign (tmp2, NOP_EXPR, tmp1);
+ gsi_insert_after (gsi, assign, GSI_NEW_STMT);
+ assign = gimple_build_assign (result, gimple_assign_lhs (assign));
When you use a temporary tmp2 for the lhs of the conversion, you can just
use it here,
assign = gimple_build_assign (result, tmp2);
Ok for trunk with that change.
Just a question, could I also use
tree tmp2 = make_temp_ssa_name (TREE_TYPE (result), NULL, name);
?
This make_temp_ssa_name() is used throughout the file and the new
make_ssa_name() would be the first use in this file.
--
embedded brains GmbH
Herr Sebastian HUBER
Dornierstr. 4
82178 Puchheim
Germany
email: sebastian.hu...@embedded-brains.de
phone: +49-89-18 94 741 - 16
fax: +49-89-18 94 741 - 08
Registergericht: Amtsgericht München
Registernummer: HRB 157899
Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
Unsere Datenschutzerklärung finden Sie hier:
https://embedded-brains.de/datenschutzerklaerung/
Re: [PATCH v2] gcov: Fix integer types in gen_counter_update()
On Tue, Nov 21, 2023 at 11:29:58AM +0100, Sebastian Huber wrote:
> This change fixes issues like this:
>
> gcc.dg/gomp/pr27573.c: In function ‘main._omp_fn.0’:
> gcc.dg/gomp/pr27573.c:19:1: error: non-trivial conversion in ‘ssa_name’
> 19 | }
> | ^
> long int
> long unsigned int
> # .MEM_19 = VDEF <.MEM_18>
> __gcov7.main._omp_fn.0[0] = PROF_time_profile_12;
> during IPA pass: profile
> gcc.dg/gomp/pr27573.c:19:1: internal compiler error: verify_gimple failed
>
> gcc/ChangeLog:
>
> PR middle-end/112634
>
> * tree-profile.cc (gen_assign_counter_update): Cast the unsigned result
> type of
> __atomic_add_fetch() to the signed counter type.
> (gen_counter_update): Fix formatting.
> --- a/gcc/tree-profile.cc
> +++ b/gcc/tree-profile.cc
> @@ -281,10 +281,13 @@ gen_assign_counter_update (gimple_stmt_iterator *gsi,
> gcall *call, tree func,
>if (result)
> {
>tree result_type = TREE_TYPE (TREE_TYPE (func));
> - tree tmp = make_temp_ssa_name (result_type, NULL, name);
> - gimple_set_lhs (call, tmp);
> + tree tmp1 = make_temp_ssa_name (result_type, NULL, name);
> + gimple_set_lhs (call, tmp1);
>gsi_insert_after (gsi, call, GSI_NEW_STMT);
> - gassign *assign = gimple_build_assign (result, tmp);
> + tree tmp2 = make_ssa_name (TREE_TYPE (result));
> + gassign *assign = gimple_build_assign (tmp2, NOP_EXPR, tmp1);
> + gsi_insert_after (gsi, assign, GSI_NEW_STMT);
> + assign = gimple_build_assign (result, gimple_assign_lhs (assign));
When you use a temporary tmp2 for the lhs of the conversion, you can just
use it here,
assign = gimple_build_assign (result, tmp2);
Ok for trunk with that change.
>gsi_insert_after (gsi, assign, GSI_NEW_STMT);
> }
>else
> @@ -309,8 +312,8 @@ gen_counter_update (gimple_stmt_iterator *gsi, tree
> counter, tree result,
> {
>/* __atomic_fetch_add (, 1, MEMMODEL_RELAXED); */
>tree f = builtin_decl_explicit (TYPE_PRECISION (type) > 32
> - ? BUILT_IN_ATOMIC_ADD_FETCH_8:
> - BUILT_IN_ATOMIC_ADD_FETCH_4);
> + ? BUILT_IN_ATOMIC_ADD_FETCH_8
> + : BUILT_IN_ATOMIC_ADD_FETCH_4);
>gcall *call = gimple_build_call (f, 3, addr, one, relaxed);
>gen_assign_counter_update (gsi, call, f, result, name);
> }
Jakub
