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.hu...@embedded-brains.de>
Sent: Wednesday, November 22, 2023 10:24 PM
To: Christophe Lyon<christophe.l...@linaro.org>
Cc: Jakub Jelinek<ja...@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.hu...@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.hu...@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 <i...@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.hu...@embedded-brains.de>
Date:   Sat Oct 21 15:52:15 2023 +0200

     gcov: Add gen_counter_update()

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

     gcov: Use unshare_expr() in gen_counter_update()

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

     gcov: Fix integer types in gen_counter_update()

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

     gcov: Improve -fprofile-update=atomic

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

     gcov: Formatting fixes

Author: Sebastian Huber <sebastian.hu...@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 <j...@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.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/

Reply via email to