On 1/8/20 11:08 AM, Jan Hubicka wrote:
On 1/7/20 11:27 AM, Martin Liška wrote:
Which is fine. Apparently there are just few usages of manual printing
of a symtab node and order like:

        fprintf (f,
             "%*s%s/%i %s\n%*s  freq:%4.2f",
             indent, "", callee->name (), callee->order,

I can replace these with symtab_node::dump_{asm_}name.

Hi.

I'm addressing this by the following refactoring patch.

Patch can bootstrap on x86_64-linux-gnu and survives regression tests.

Ready to be installed?
This is OK.

Ok, I've just installed both patches.

The original reason for using DECL_NAME rather than DECL_ASSEMBLER_NAME
was that first is shorter, but also that DECL_ASSEMBLER_NAME may trigger
lazy assembler name computation that changes memory layout and let to
divergences in resulting code when compiling with and without
-fdump-ipa-all.

Ah, that's new to me.


I guess for dumping we should use dump_name consistently unless we
really want to speak of the assembler symbol name (so I would suggest
fixing the uses of dump_asm_name in most cases to dump_name)

Well, I do prefer the asm names in cgraph dump files as heavy templated
C++ code generates function names that are much harder to grep. I know one
can use order to find these.

Example:



void Field<M, T, E>::clearDirty() const [with Mesh = UniformRectilinearMesh<MeshTraits<3, double, 
UniformRectilinearTag, CartesianTag, 3> >; T = double; EngineTag = MultiPatch<GridTag, 
Remote<Brick> >]/4590
vs.
_ZNK5FieldI22UniformRectilinearMeshI10MeshTraitsILi3Ed21UniformRectilinearTag12CartesianTagLi3EEEd10MultiPatchI7GridTag6RemoteI5BrickEEE10clearDirtyEv/4590


We should not use node->name - this is artifact from times we did not
have dump_name for dumping.

I see still about 80 usages of that in dumps. I'll prepare a replacement
patch.


Honza
Thanks,
Martin

 From 45629d68ac4ad9dada74e8c14a10b89025f54762 Mon Sep 17 00:00:00 2001
From: Martin Liska <mli...@suse.cz>
Date: Tue, 7 Jan 2020 12:37:42 +0100
Subject: [PATCH] Replace node->name/node->order with node->dump_name.

gcc/ChangeLog:

2020-01-07  Martin Liska  <mli...@suse.cz>

        * ipa-fnsummary.c (dump_ipa_call_summary): Use symtab_node::dump_name.
        (ipa_call_context::estimate_size_and_time): Likewise.
        (inline_analyze_function): Likewise.

gcc/lto/ChangeLog:

2020-01-07  Martin Liska  <mli...@suse.cz>

        * lto-partition.c (lto_balanced_map): Use symtab_node::dump_name.
---
  gcc/ipa-fnsummary.c     | 12 +++++-------
  gcc/lto/lto-partition.c |  4 ++--
  2 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/gcc/ipa-fnsummary.c b/gcc/ipa-fnsummary.c
index fa01cb6c083..7c0b6f98e25 100644
--- a/gcc/ipa-fnsummary.c
+++ b/gcc/ipa-fnsummary.c
@@ -907,8 +907,8 @@ dump_ipa_call_summary (FILE *f, int indent, struct 
cgraph_node *node,
        int i;
fprintf (f,
-              "%*s%s/%i %s\n%*s  freq:%4.2f",
-              indent, "", callee->name (), callee->order,
+              "%*s%s %s\n%*s  freq:%4.2f",
+              indent, "", callee->dump_name (),
               !edge->inline_failed
               ? "inlined" : cgraph_inline_failed_string (edge-> inline_failed),
               indent, "", edge->sreal_frequency ().to_double ());
@@ -3505,9 +3505,8 @@ ipa_call_context::estimate_size_and_time (int *ret_size,
    if (dump_file && (dump_flags & TDF_DETAILS))
      {
        bool found = false;
-      fprintf (dump_file, "   Estimating body: %s/%i\n"
-              "   Known to be false: ", m_node->name (),
-              m_node->order);
+      fprintf (dump_file, "   Estimating body: %s\n"
+              "   Known to be false: ", m_node->dump_name ());
for (i = predicate::not_inlined_condition;
           i < (predicate::first_dynamic_condition
@@ -4034,8 +4033,7 @@ inline_analyze_function (struct cgraph_node *node)
    push_cfun (DECL_STRUCT_FUNCTION (node->decl));
if (dump_file)
-    fprintf (dump_file, "\nAnalyzing function: %s/%u\n",
-            node->name (), node->order);
+    fprintf (dump_file, "\nAnalyzing function: %s\n", node->dump_name ());
    if (opt_for_fn (node->decl, optimize) && !node->thunk.thunk_p)
      inline_indirect_intraprocedural_analysis (node);
    compute_fn_summary (node, false);
diff --git a/gcc/lto/lto-partition.c b/gcc/lto/lto-partition.c
index 86b2eabe374..5b153c9759e 100644
--- a/gcc/lto/lto-partition.c
+++ b/gcc/lto/lto-partition.c
@@ -734,10 +734,10 @@ lto_balanced_map (int n_lto_partitions, int 
max_partition_size)
          best_varpool_pos = varpool_pos;
        }
        if (dump_file)
-       fprintf (dump_file, "Step %i: added %s/%i, size %i, "
+       fprintf (dump_file, "Step %i: added %s, size %i, "
                 "cost %" PRId64 "/%" PRId64 " "
                 "best %" PRId64 "/%" PRId64", step %i\n", i,
-                order[i]->name (), order[i]->order,
+                order[i]->dump_name (),
                 partition->insns, cost, internal,
                 best_cost, best_internal, best_i);
        /* Partition is too large, unwind into step when best cost was reached 
and
--
2.24.1



Reply via email to