public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH v2] gcov: Fix integer types in gen_counter_update()
@ 2023-11-21 10:29 Sebastian Huber
  2023-11-21 10:34 ` Jakub Jelinek
  0 siblings, 1 reply; 11+ messages in thread
From: Sebastian Huber @ 2023-11-21 10:29 UTC (permalink / raw)
  To: gcc-patches; +Cc: Jakub Jelinek, Dimitar Dimitrov, Tobias Burnus

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.
---

v2: Use NOP_EXPR to do the cast.  Fix formatting.

 gcc/tree-profile.cc | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/gcc/tree-profile.cc b/gcc/tree-profile.cc
index f12b374ca27..ff95f8ef7cd 100644
--- 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));
       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 (&counter, 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);
     }
-- 
2.35.3


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] gcov: Fix integer types in gen_counter_update()
  2023-11-21 10:29 [PATCH v2] gcov: Fix integer types in gen_counter_update() Sebastian Huber
@ 2023-11-21 10:34 ` Jakub Jelinek
  2023-11-21 10:42   ` Sebastian Huber
  0 siblings, 1 reply; 11+ messages in thread
From: Jakub Jelinek @ 2023-11-21 10:34 UTC (permalink / raw)
  To: Sebastian Huber; +Cc: gcc-patches, Dimitar Dimitrov, Tobias Burnus

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 (&counter, 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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] gcov: Fix integer types in gen_counter_update()
  2023-11-21 10:34 ` Jakub Jelinek
@ 2023-11-21 10:42   ` Sebastian Huber
  2023-11-21 10:46     ` Jakub Jelinek
  0 siblings, 1 reply; 11+ messages in thread
From: Sebastian Huber @ 2023-11-21 10:42 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-patches



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.huber@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/

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] gcov: Fix integer types in gen_counter_update()
  2023-11-21 10:42   ` Sebastian Huber
@ 2023-11-21 10:46     ` Jakub Jelinek
  2023-11-21 11:21       ` Sebastian Huber
  0 siblings, 1 reply; 11+ messages in thread
From: Jakub Jelinek @ 2023-11-21 10:46 UTC (permalink / raw)
  To: Sebastian Huber; +Cc: gcc-patches

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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] gcov: Fix integer types in gen_counter_update()
  2023-11-21 10:46     ` Jakub Jelinek
@ 2023-11-21 11:21       ` Sebastian Huber
  2023-11-22 14:22         ` Christophe Lyon
  0 siblings, 1 reply; 11+ messages in thread
From: Sebastian Huber @ 2023-11-21 11:21 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-patches

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.huber@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/

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] gcov: Fix integer types in gen_counter_update()
  2023-11-21 11:21       ` Sebastian Huber
@ 2023-11-22 14:22         ` Christophe Lyon
  2023-11-22 14:24           ` Sebastian Huber
  0 siblings, 1 reply; 11+ messages in thread
From: Christophe Lyon @ 2023-11-22 14:22 UTC (permalink / raw)
  To: Sebastian Huber; +Cc: Jakub Jelinek, gcc-patches

Hi,

On Tue, 21 Nov 2023 at 12:22, Sebastian Huber
<sebastian.huber@embedded-brains.de> 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.huber@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/

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] gcov: Fix integer types in gen_counter_update()
  2023-11-22 14:22         ` Christophe Lyon
@ 2023-11-22 14:24           ` Sebastian Huber
  2023-11-23  8:11             ` Jiang, Haochen
  0 siblings, 1 reply; 11+ messages in thread
From: Sebastian Huber @ 2023-11-22 14:24 UTC (permalink / raw)
  To: Christophe Lyon; +Cc: Jakub Jelinek, gcc-patches

On 22.11.23 15:22, Christophe Lyon wrote:
> On Tue, 21 Nov 2023 at 12:22, Sebastian Huber
> <sebastian.huber@embedded-brains.de>  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.huber@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/

^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [PATCH v2] gcov: Fix integer types in gen_counter_update()
  2023-11-22 14:24           ` Sebastian Huber
@ 2023-11-23  8:11             ` Jiang, Haochen
  2023-11-23  8:20               ` Sebastian Huber
  0 siblings, 1 reply; 11+ messages in thread
From: Jiang, Haochen @ 2023-11-23  8:11 UTC (permalink / raw)
  To: Sebastian Huber, Christophe Lyon; +Cc: Jakub Jelinek, gcc-patches

> -----Original Message-----
> From: Sebastian Huber <sebastian.huber@embedded-brains.de>
> Sent: Wednesday, November 22, 2023 10:24 PM
> To: Christophe Lyon <christophe.lyon@linaro.org>
> Cc: Jakub Jelinek <jakub@redhat.com>; 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
> > <sebastian.huber@embedded-brains.de>  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.huber@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/

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] gcov: Fix integer types in gen_counter_update()
  2023-11-23  8:11             ` Jiang, Haochen
@ 2023-11-23  8:20               ` Sebastian Huber
  2023-11-23 17:29                 ` Sebastian Huber
  0 siblings, 1 reply; 11+ messages in thread
From: Sebastian Huber @ 2023-11-23  8:20 UTC (permalink / raw)
  To: Jiang, Haochen, Christophe Lyon; +Cc: Jakub Jelinek, gcc-patches

On 23.11.23 09:11, Jiang, Haochen wrote:
>> -----Original Message-----
>> From: Sebastian Huber<sebastian.huber@embedded-brains.de>
>> Sent: Wednesday, November 22, 2023 10:24 PM
>> To: Christophe Lyon<christophe.lyon@linaro.org>
>> Cc: Jakub Jelinek<jakub@redhat.com>;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
>>> <sebastian.huber@embedded-brains.de>   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.huber@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/

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] gcov: Fix integer types in gen_counter_update()
  2023-11-23  8:20               ` Sebastian Huber
@ 2023-11-23 17:29                 ` Sebastian Huber
  2023-11-24 14:36                   ` Sebastian Huber
  0 siblings, 1 reply; 11+ messages in thread
From: Sebastian Huber @ 2023-11-23 17:29 UTC (permalink / raw)
  To: Jiang, Haochen, Christophe Lyon; +Cc: Jakub Jelinek, gcc-patches

On 23.11.23 09:20, Sebastian Huber wrote:
> On 23.11.23 09:11, Jiang, Haochen wrote:
>>> -----Original Message-----
>>> From: Sebastian Huber<sebastian.huber@embedded-brains.de>
>>> Sent: Wednesday, November 22, 2023 10:24 PM
>>> To: Christophe Lyon<christophe.lyon@linaro.org>
>>> Cc: Jakub Jelinek<jakub@redhat.com>;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
>>>> <sebastian.huber@embedded-brains.de>   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 <sebastian.huber@embedded-brains.de>
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 <iain@sandoe.co.uk>
Date:   Tue Nov 21 10:19:29 2023 +0000

     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 <sebastian.huber@embedded-brains.de>
Date:   Sat Oct 21 15:52:15 2023 +0200

     gcov: Add gen_counter_update()

Author: Sebastian Huber <sebastian.huber@embedded-brains.de>
Date:   Mon Nov 20 14:48:03 2023 +0100

     gcov: Use unshare_expr() in gen_counter_update()

Author: Sebastian Huber <sebastian.huber@embedded-brains.de>
Date:   Mon Nov 20 15:26:38 2023 +0100

     gcov: Fix integer types in gen_counter_update()

Author: Sebastian Huber <sebastian.huber@embedded-brains.de>
Date:   Tue Nov 14 21:36:51 2023 +0100

     gcov: Improve -fprofile-update=atomic

Author: Jakub Jelinek <jakub@redhat.com>
Date:   Tue Nov 21 10:49:51 2023 +0100

     gcov: Formatting fixes

Author: Sebastian Huber <sebastian.huber@embedded-brains.de>
Date:   Thu Nov 23 06:44:15 2023 +0000

     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/src/.libs:/home/sh/b-gcc/aarch64-unknown-linux-gnu/libvtv/.libs:/home/sh/b-gcc/aarch64-unknown-linux-gnu/libssp/.libs:/home/sh/b-gcc/aarch64-unknown-linux-gnu/libgomp/.libs:/home/sh/b-gcc/aarch64-unknown-linux-gnu/libitm/.libs:/home/sh/b-gcc/aarch64-unknown-linux-gnu/libatomic/.libs:/home/sh/b-gcc/./gcc:/home/sh/b-gcc/./prev-gcc:/home/sh/b-gcc/aarch64-unknown-linux-gnu/libstdc++-v3/src/.libs:/home/sh/b-gcc/aarch64-unknown-linux-gnu/libvtv/.libs:/home/sh/b-gcc/aarch64-unknown-linux-gnu/libssp/.libs:/home/sh/b-gcc/aarch64-unknown-linux-gnu/libgomp/.libs:/home/sh/b-gcc/aarch64-unknown-linux-gnu/libitm/.libs:/home/sh/b-gcc/aarch64-unknown-linux-gnu/libatomic/.libs:/home/sh/b-gcc/./gcc:/home/sh/b-gcc/./prev-gcc
Execution timeout is: 300
spawn [open ...]
PASS: gcc.dg/tree-prof/time-profiler-3.c execution, 
-fprofile-generate -D_PROFILE_GENERATE
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-use -D_PROFILE_USE -dumpbase-ext .x02 
-lm  -o /home/sh/b-gcc/gcc/testsuite/gcc5/time-profiler-3.x02 
(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-use -D_PROFILE_USE -dumpbase-ext .x02 
-lm -o /home/sh/b-gcc/gcc/testsuite/gcc5/time-profiler-3.x02
PASS: gcc.dg/tree-prof/time-profiler-3.c compilation,  -fprofile-use 
-D_PROFILE_USE
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/src/.libs:/home/sh/b-gcc/aarch64-unknown-linux-gnu/libvtv/.libs:/home/sh/b-gcc/aarch64-unknown-linux-gnu/libssp/.libs:/home/sh/b-gcc/aarch64-unknown-linux-gnu/libgomp/.libs:/home/sh/b-gcc/aarch64-unknown-linux-gnu/libitm/.libs:/home/sh/b-gcc/aarch64-unknown-linux-gnu/libatomic/.libs:/home/sh/b-gcc/./gcc:/home/sh/b-gcc/./prev-gcc:/home/sh/b-gcc/aarch64-unknown-linux-gnu/libstdc++-v3/src/.libs:/home/sh/b-gcc/aarch64-unknown-linux-gnu/libvtv/.libs:/home/sh/b-gcc/aarch64-unknown-linux-gnu/libssp/.libs:/home/sh/b-gcc/aarch64-unknown-linux-gnu/libgomp/.libs:/home/sh/b-gcc/aarch64-unknown-linux-gnu/libitm/.libs:/home/sh/b-gcc/aarch64-unknown-linux-gnu/libatomic/.libs:/home/sh/b-gcc/./gcc:/home/sh/b-gcc/./prev-gcc
Execution timeout is: 300
spawn [open ...]
PASS: gcc.dg/tree-prof/time-profiler-3.c execution,    -fprofile-use 
-D_PROFILE_USE
PASS: gcc.dg/tree-prof/time-profiler-3.c scan-ipa-dump-times profile 
"Read tp_first_run: 0" 1
PASS: gcc.dg/tree-prof/time-profiler-3.c scan-ipa-dump-times profile 
"Read tp_first_run: 1" 1
PASS: gcc.dg/tree-prof/time-profiler-3.c scan-ipa-dump-times profile 
"Read tp_first_run: 2" 1

I will try to bisect this tomorrow.

-- 
embedded brains GmbH
Herr Sebastian HUBER
Dornierstr. 4
82178 Puchheim
Germany
email: sebastian.huber@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/

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] gcov: Fix integer types in gen_counter_update()
  2023-11-23 17:29                 ` Sebastian Huber
@ 2023-11-24 14:36                   ` Sebastian Huber
  0 siblings, 0 replies; 11+ messages in thread
From: Sebastian Huber @ 2023-11-24 14:36 UTC (permalink / raw)
  To: Jiang, Haochen, Christophe Lyon, Jan Hubicka; +Cc: Jakub Jelinek, gcc-patches

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<sebastian.huber@embedded-brains.de>
>>>> Sent: Wednesday, November 22, 2023 10:24 PM
>>>> To: Christophe Lyon<christophe.lyon@linaro.org>
>>>> Cc: Jakub Jelinek<jakub@redhat.com>;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
>>>>> <sebastian.huber@embedded-brains.de>   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 <sebastian.huber@embedded-brains.de>
> 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 <iain@sandoe.co.uk>
> Date:   Tue Nov 21 10:19:29 2023 +0000
> 
>      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 <sebastian.huber@embedded-brains.de>
> Date:   Sat Oct 21 15:52:15 2023 +0200
> 
>      gcov: Add gen_counter_update()
> 
> Author: Sebastian Huber <sebastian.huber@embedded-brains.de>
> Date:   Mon Nov 20 14:48:03 2023 +0100
> 
>      gcov: Use unshare_expr() in gen_counter_update()
> 
> Author: Sebastian Huber <sebastian.huber@embedded-brains.de>
> Date:   Mon Nov 20 15:26:38 2023 +0100
> 
>      gcov: Fix integer types in gen_counter_update()
> 
> Author: Sebastian Huber <sebastian.huber@embedded-brains.de>
> Date:   Tue Nov 14 21:36:51 2023 +0100
> 
>      gcov: Improve -fprofile-update=atomic
> 
> Author: Jakub Jelinek <jakub@redhat.com>
> Date:   Tue Nov 21 10:49:51 2023 +0100
> 
>      gcov: Formatting fixes
> 
> Author: Sebastian Huber <sebastian.huber@embedded-brains.de>
> Date:   Thu Nov 23 06:44:15 2023 +0000
> 
>      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/src/.libs:/home/sh/b-gcc/aarch64-unknown-linux-gnu/libvtv/.libs:/home/sh/b-gcc/aarch64-unknown-linux-gnu/libssp/.libs:/home/sh/b-gcc/aarch64-unknown-linux-gnu/libgomp/.libs:/home/sh/b-gcc/aarch64-unknown-linux-gnu/libitm/.libs:/home/sh/b-gcc/aarch64-unknown-linux-gnu/libatomic/.libs:/home/sh/b-gcc/./gcc:/home/sh/b-gcc/./prev-gcc:/home/sh/b-gcc/aarch64-unknown-linux-gnu/libstdc++-v3/src/.libs:/home/sh/b-gcc/aarch64-unknown-linux-gnu/libvtv/.libs:/home/sh/b-gcc/aarch64-unknown-linux-gnu/libssp/.libs:/home/sh/b-gcc/aarch64-unknown-linux-gnu/libgomp/.libs:/home/sh/b-gcc/aarch64-unknown-linux-gnu/libitm/.libs:/home/sh/b-gcc/aarch64-unknown-linux-gnu/libatomic/.libs:/home/sh/b-gcc/./gcc:/home/sh/b-gcc/./prev-gcc
> Execution timeout is: 300
> spawn [open ...]
> PASS: gcc.dg/tree-prof/time-profiler-3.c execution, -fprofile-generate 
> -D_PROFILE_GENERATE
> 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-use -D_PROFILE_USE -dumpbase-ext .x02 
> -lm  -o /home/sh/b-gcc/gcc/testsuite/gcc5/time-profiler-3.x02 (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-use -D_PROFILE_USE -dumpbase-ext .x02 
> -lm -o /home/sh/b-gcc/gcc/testsuite/gcc5/time-profiler-3.x02
> PASS: gcc.dg/tree-prof/time-profiler-3.c compilation,  -fprofile-use 
> -D_PROFILE_USE
> 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/src/.libs:/home/sh/b-gcc/aarch64-unknown-linux-gnu/libvtv/.libs:/home/sh/b-gcc/aarch64-unknown-linux-gnu/libssp/.libs:/home/sh/b-gcc/aarch64-unknown-linux-gnu/libgomp/.libs:/home/sh/b-gcc/aarch64-unknown-linux-gnu/libitm/.libs:/home/sh/b-gcc/aarch64-unknown-linux-gnu/libatomic/.libs:/home/sh/b-gcc/./gcc:/home/sh/b-gcc/./prev-gcc:/home/sh/b-gcc/aarch64-unknown-linux-gnu/libstdc++-v3/src/.libs:/home/sh/b-gcc/aarch64-unknown-linux-gnu/libvtv/.libs:/home/sh/b-gcc/aarch64-unknown-linux-gnu/libssp/.libs:/home/sh/b-gcc/aarch64-unknown-linux-gnu/libgomp/.libs:/home/sh/b-gcc/aarch64-unknown-linux-gnu/libitm/.libs:/home/sh/b-gcc/aarch64-unknown-linux-gnu/libatomic/.libs:/home/sh/b-gcc/./gcc:/home/sh/b-gcc/./prev-gcc
> Execution timeout is: 300
> spawn [open ...]
> PASS: gcc.dg/tree-prof/time-profiler-3.c execution,    -fprofile-use 
> -D_PROFILE_USE
> PASS: gcc.dg/tree-prof/time-profiler-3.c scan-ipa-dump-times profile 
> "Read tp_first_run: 0" 1
> PASS: gcc.dg/tree-prof/time-profiler-3.c scan-ipa-dump-times profile 
> "Read tp_first_run: 1" 1
> PASS: gcc.dg/tree-prof/time-profiler-3.c scan-ipa-dump-times profile 
> "Read tp_first_run: 2" 1
> 
> I will try to bisect this tomorrow.
> 

I rebased the failing master on top of this modified branch with the 
gcov patches. Then I performed a git bisect. It found this commit:

commit 53ba8d669550d3a1f809048428b97ca607f95cf5
Author: Jan Hubicka <jh@suse.cz>
Date:   Mon Nov 20 19:35:53 2023 +0100

     inter-procedural value range propagation

     implement very basic propapgation of return value ranges from VRP
     pass.  This helps std::vector's push_back since we work out value 
range of
     allocated block.  This propagates only within single translation 
unit.  I hoped
     we will also do the propagation at WPA stage, but that needs more 
work on
     ipa-cp side.

     I also added code auto-detecting return_nonnull and corresponding 
-Wsuggest-attribute.

In the change log we have:

             * gcc.dg/tree-prof/time-profiler-1.c: Disable ipa-vrp.
             * gcc.dg/tree-prof/time-profiler-2.c: Disable ipa-vrp.

The gcc.dg/tree-prof/time-profiler-3.c doesn't show up in the change 
log. Maybe this is the problem.

-- 
embedded brains GmbH & Co. KG
Herr Sebastian HUBER
Dornierstr. 4
82178 Puchheim
Germany
email: sebastian.huber@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/

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2023-11-24 14:36 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-21 10:29 [PATCH v2] gcov: Fix integer types in gen_counter_update() Sebastian Huber
2023-11-21 10:34 ` Jakub Jelinek
2023-11-21 10:42   ` Sebastian Huber
2023-11-21 10:46     ` Jakub Jelinek
2023-11-21 11:21       ` Sebastian Huber
2023-11-22 14:22         ` Christophe Lyon
2023-11-22 14:24           ` Sebastian Huber
2023-11-23  8:11             ` Jiang, Haochen
2023-11-23  8:20               ` Sebastian Huber
2023-11-23 17:29                 ` Sebastian Huber
2023-11-24 14:36                   ` Sebastian Huber

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).