Hi,
I was trying to address first TODO from ipa-comdats.c (attached patch)
TODO: When symbol is used only by comdat symbols, but from different groups,
it would make sense to produce a new comdat group for it with anonymous name.
The patch introduces new lattice value ANON "between" COMDAT and BOTTOM values.
The meet operation is rewritten in merge_comdat_group().
Does the approach look reasonable ?
For comdat-1.C (attached) q is referenced from both i1 and i2
and hence is put in it's own comdat-section.
I suppose that's the expected result ?
However it fails for comdat-2.C with the error below.
error: comdat-local function called by int i1() outside its comdat
However in case of comdat-1.C, i1() calls q() which is also placed
in another comdat group but doesn't give error for that case.
It also works if r() is referenced from another comdat group
and not just from q (comdat-3.C).
I am not able to understand why the error occurs for comdat-2.C.
Could someone please have a look at it ?
Thanks!
Assembling functions:
comdat-2.C: At global scope:
comdat-2.C:33:15: error: comdat-local function called by int i2()
outside its comdat
int (*f2)()=i2;
^
comdat-2.C:33:15: error: comdat-local function called by int i1()
outside its comdat
_ZL1qv/1 (int q()) @0x7fe19ccde170
Type: function definition analyzed
Visibility: prevailing_def_ironly comdat_group:q
Same comdat group as: _ZL1rv/0
References:
Referring:
Availability: local
First run: 0
Function flags: body local
Called by: _Z2i2v/3 (1.00 per call) (can throw external) _Z2i1v/2
(1.00 per call) (can throw external)
Calls: _ZL1rv/0 (1.00 per call) (can throw external) _Z6printfPKcz/6
(1.00 per call) (can throw external)
comdat-2.C:33:15: internal compiler error: verify_cgraph_node failed
0x936083 cgraph_node::verify_node()
../../gcc/gcc/cgraph.c:3233
0x92b6df symtab_node::verify()
../../gcc/gcc/symtab.c:1177
0x92b7cf symtab_node::verify_symtab_nodes()
../../gcc/gcc/symtab.c:1197
0x93e736 symtab_node::checking_verify_symtab_nodes()
../../gcc/gcc/cgraph.h:602
0x93e736 symbol_table::compile()
../../gcc/gcc/cgraphunit.c:2431
0x940a57 symbol_table::compile()
../../gcc/gcc/cgraphunit.c:2538
0x940a57 symbol_table::finalize_compilation_unit()
../../gcc/gcc/cgraphunit.c:2564
Thanks,
Prathamesh
diff --git a/gcc/ipa-comdats.c b/gcc/ipa-comdats.c
index 6e0f762..999bed2 100644
--- a/gcc/ipa-comdats.c
+++ b/gcc/ipa-comdats.c
@@ -56,6 +56,68 @@ along with GCC; see the file COPYING3. If not see
#include "tree-pass.h"
#include "cgraph.h"
+
+static void
+set_anon (tree& newgroup, symtab_node *symbol,
+ hash_set<tree>& anon_set,
+ hash_map<tree, symtab_node *>& comdat_head_map)
+{
+ newgroup = DECL_NAME (symbol->decl);
+ anon_set.add (newgroup);
+ comdat_head_map.put (newgroup, symbol);
+ symbol->set_comdat_group (newgroup);
+ fprintf (stderr, "newgroup goes to anon: %s\n", IDENTIFIER_POINTER
(newgroup));
+}
+
+static void
+merge_comdat_group (tree& newgroup, tree *val2, symtab_node *symbol,
+ hash_set<tree>& anon_set,
+ hash_map<tree, symtab_node *>& comdat_head_map)
+{
+ /* *val2 is TOP. */
+ if (val2 == NULL || *val2 == NULL)
+ return;
+
+ /* *val2 is BOTTOM. */
+ if (*val2 == error_mark_node)
+ {
+ newgroup = error_mark_node;
+ return;
+ }
+
+ /* *val2 is ANON. */
+ else if (anon_set.contains (*val2))
+ {
+ /* ANON meet TOP == ANON. */
+ if (newgroup == NULL)
+ newgroup = *val2;
+ /* ANON1 meet ANON2 == ANON2. */
+ else if (anon_set.contains (newgroup))
+ ;
+ /* ANON meet COMDAT == ANON2. */
+ else
+ set_anon (newgroup, symbol, anon_set, comdat_head_map);
+ }
+
+ /* *val2 is COMDAT. */
+ else
+ {
+ /* COMDAT meet TOP == COMDAT. */
+ if (newgroup == NULL)
+ {
+ newgroup = *val2;
+ return;
+ }
+ /* COMDAT1 meet COMDAT2 == ANON. */
+ else if (*val2 != newgroup)
+ set_anon (newgroup, symbol, anon_set, comdat_head_map);
+ /* ensure this case is COMDAT meet COMDAT. */
+ else
+ gcc_assert (*val2 == newgroup);
+ }
+}
+
+
/* Main dataflow loop propagating comdat groups across
the symbol table. All references to SYMBOL are examined
and NEWGROUP is updated accordingly. MAP holds current lattice
@@ -63,7 +125,9 @@ along with GCC; see the file COPYING3. If not see
tree
propagate_comdat_group (struct symtab_node *symbol,
- tree newgroup, hash_map<symtab_node *, tree> &map)
+ tree newgroup, hash_map<symtab_node *, tree> &map,
+ hash_set<tree>& anon_set,
+ hash_map<tree, symtab_node *>& comdat_head_map)
{
int i;
struct ipa_ref *ref;
@@ -78,7 +142,7 @@ propagate_comdat_group (struct symtab_node *symbol,
if (ref->use == IPA_REF_ALIAS)
{
- newgroup = propagate_comdat_group (symbol2, newgroup, map);
+ newgroup = propagate_comdat_group (symbol2, newgroup, map, anon_set,
comdat_head_map);
continue;
}
@@ -102,17 +166,7 @@ propagate_comdat_group (struct symtab_node *symbol,
symbol2 = cn->global.inlined_to;
}
- /* The actual merge operation. */
-
- tree *val2 = map.get (symbol2);
-
- if (val2 && *val2 != newgroup)
- {
- if (!newgroup)
- newgroup = *val2;
- else
- newgroup = error_mark_node;
- }
+ merge_comdat_group (newgroup, map.get (symbol2), symbol, anon_set,
comdat_head_map);
}
/* If we analyze function, walk also callers. */
@@ -129,7 +183,7 @@ propagate_comdat_group (struct symtab_node *symbol,
{
/* Thunks can not call across section boundary. */
if (cn->thunk.thunk_p)
- newgroup = propagate_comdat_group (symbol2, newgroup, map);
+ newgroup = propagate_comdat_group (symbol2, newgroup, map,
anon_set, comdat_head_map);
/* If we see inline clone, its comdat group actually
corresponds to the comdat group of the function it
is inlined to. */
@@ -137,17 +191,7 @@ propagate_comdat_group (struct symtab_node *symbol,
symbol2 = cn->global.inlined_to;
}
- /* The actual merge operation. */
-
- tree *val2 = map.get (symbol2);
-
- if (val2 && *val2 != newgroup)
- {
- if (!newgroup)
- newgroup = *val2;
- else
- newgroup = error_mark_node;
- }
+ merge_comdat_group (newgroup, map.get (symbol2), symbol, anon_set,
comdat_head_map);
}
return newgroup;
}
@@ -237,6 +280,7 @@ ipa_comdats (void)
bool comdat_group_seen = false;
symtab_node *first = (symtab_node *) (void *) 1;
tree group;
+ hash_set<tree> anon_set;
/* Start the dataflow by assigning comdat group to symbols that are in comdat
groups already. All other externally visible symbols must stay, we use
@@ -310,7 +354,7 @@ ipa_comdats (void)
if (group == error_mark_node)
continue;
- newgroup = propagate_comdat_group (symbol, group, map);
+ newgroup = propagate_comdat_group (symbol, group, map, anon_set,
comdat_head_map);
/* If nothing changed, proceed to next symbol. */
if (newgroup == group)
/* { dg-do compile { target *-*-linux* *-*-gnu* } } */
/* { dg-options "-O2 -fdump-ipa-comdats" } */
int printf (const char *, ...);
/* q should be placed in anon comdat group. */
__attribute__ ((noinline))
static int q(void)
{
printf ("test");
}
inline int i1(void)
{
return q();
}
inline int i2(void)
{
return q();
}
int (*f)()=i1;
int (*f2)()=i2;
/* { dg-final { scan-ipa-dump-times "Localizing symbol" 1 "comdats" } } */
/* { dg-do compile { target *-*-linux* *-*-gnu* } } */
/* { dg-options "-O2 -fdump-ipa-comdats" } */
/* q should be put in anonymous comdat group since it's referenced from i1 and i2.
r should be put in same comdat group as q. */
int printf (const char *, ...);
__attribute__ ((noinline))
static int r(void)
{
return printf ("world");
}
__attribute__ ((noinline))
static int q(void)
{
printf ("test");
return r();
}
inline int i1(void)
{
return q();
}
inline int i2(void)
{
return q();
}
int (*f)()=i1;
int (*f2)()=i2;
/* { dg-final { scan-ipa-dump-times "Localizing symbol" 1 "comdats" } } */
/* { dg-do compile { target *-*-linux* *-*-gnu* } } */
/* { dg-options "-O2 -fdump-ipa-comdats" } */
int printf (const char *, ...);
/* q and r should be placed in anonymous groups. */
__attribute__ ((noinline))
static int r(void)
{
return printf ("world");
}
__attribute__ ((noinline))
static int q(void)
{
printf ("test");
return r();
}
inline int i1(void)
{
return q();
}
inline int i2(void)
{
return q();
}
inline int i3(void)
{
return r();
}
int (*f)()=i1;
int (*f2)()=i2;
int (*f3)()=i3;
/* { dg-final { scan-ipa-dump-times "Localizing symbol" 1 "comdats" } } */
