Re: [PATCH] cgraph: remove dead if stmt in build_cgraph_edges pass

[Josef Melcr]

So I experimented a little and ran the testsuite a few times. While both if statements seem to be dead, the assertion gcc_checking_assert (!is_gimple_omp (stmt)) doesn't actually hold, as adding this assert breaks around 40 omp/oacc tests, so some other statements are definitely slipping through. I would need to dig deeper to see which statements those are. I am not quite sure where that leaves this patch.

Best regards,

Josef Melcr

Dne 24. 10. 24 v 19:45 Josef Melcr napsal(a):

Capital Remove
The second line should be just tab indented, not tab + 2 spaces, and
finished with dot.  gomp_parallel rather than gomp-parallel.

Sorry about the formatting issues, I didn't notice them.

The if (gimple_code (stmt) == GIMPLE_OMP_TASK) case should go as well.

would be needed, we miss various other GIMPLE_OMP_* statements).

Wonder if gcc_checking_assert (!is_gimple_omp (stmt)); wouldn't be useful as replacement at least for some time to verify it isn't needed (but if it

The GIMPLE_OMP_TASK case went under my radar, since I was primarily focused on gomp_parallel statements.  I'll try these changes and retest.

Best regards,

Josef Melcr

Dne 24. 10. 24 v 18:49 Jakub Jelinek napsal(a):

On Thu, Oct 24, 2024 at 04:37:24PM +0200, Josef Melcr wrote:

This patch removes a dead if statement checking for gomp-parallel gimple statements. This if is in the execute method of build_cgraph_edges pass,

which is executed right after the omp_expand pass, which removes these
gimple statements and replaces them with simple gcalls, making this if
practically dead.

Some TSan tests are failing with this patch, but I don't think this

not consistent across runs, making me think these failures are a


OK for master ?

gcc/ChangeLog:

    * cgraphbuild.cc (pass_build_cgraph_edges::execute): remove if

change is likely to cause these failures. Additionally, the failures are bug in TSan itself. All other tests are ok. Tested on x86_64-pc-linux-gnu.

Capital Remove

      statement checking for gomp-parallel statements

The second line should be just tab indented, not tab + 2 spaces, and
finished with dot.  gomp_parallel rather than gomp-parallel.

--- a/gcc/cgraphbuild.cc
+++ b/gcc/cgraphbuild.cc
@@ -340,12 +340,6 @@ pass_build_cgraph_edges::execute (function *fun)
                          bb->count);
          }
        node->record_stmt_references (stmt);

-        {
-          tree fn = gimple_omp_parallel_child_fn (omp_par_stmt);
-          node->create_reference (cgraph_node::get_create (fn),
-                      IPA_REF_ADDR, stmt);
-        }
        if (gimple_code (stmt) == GIMPLE_OMP_TASK)
          {
            tree fn = gimple_omp_task_child_fn (stmt);

-      if (gomp_parallel *omp_par_stmt = dyn_cast <gomp_parallel *> (stmt))

The if (gimple_code (stmt) == GIMPLE_OMP_TASK) case should go as well.

would be needed, we miss various other GIMPLE_OMP_* statements).

    Jakub

Wonder if gcc_checking_assert (!is_gimple_omp (stmt)); wouldn't be useful as replacement at least for some time to verify it isn't needed (but if it

smime.p7s
Description: Elektronicky podpis S/MIME