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 simply puts symbol in a new comdat group and makes symbol
the head of that group if newgroup and *val2 are COMDAT but not equal
instead of setting newgroup to BOTTOM.
Does this approach look reasonable ?
For test-1.C (attached) q() is referenced from i1() and i2()
which are comdat symbols and hence q() is put in it's own comdat-section.
I suppose that's the expected result ?
However it fails for test-2.C (attached) with the error
error: comdat-local function called by int i1() outside its comdat
and ICE verify_cgraph_node failed follows, which comes from
cgraph.c:3095:
bool check_comdat = comdat_local_p ();
if (check_comdat
&& !in_same_comdat_group_p (e->caller))
{
error ("comdat-local function called by %s outside its comdat",
identifier_to_locale (e->caller->name ()));
error_found = true;
}
Patch works for test-1.C because although q() is in different comdat
group from it's
callers, it's the only function in that group and hence
same_comdat_group is NULL
so comdat_local_p() returns false. Since check_comdat becomes false, we don't
hit the error.
For test-2.C, since r() is called by q(), the patch puts r() and q() in same
comdat group with name "q".
In this case for q(), comdat_local_p() returns true, because
same_comdat_group is non-NULL.
Since check_comdat is true and q() and it's caller i1() are not in
same comdat groups, we hit the error.
I am not sure how to fix this, and would be grateful for suggestions.
I assumed r() and q() should be in same comdat group since q() became
a comdat symbol
and r() is only referenced from q().
Also, what name would be appropriate for "anonymous" comdat group ?
I am currently giving it the name of the first symbol that gets put into it.
Thanks,
Prathamesh
diff --git a/gcc/ipa-comdats.c b/gcc/ipa-comdats.c
index 6e0f762..ea59bd9 100644
--- a/gcc/ipa-comdats.c
+++ b/gcc/ipa-comdats.c
@@ -55,6 +55,48 @@ along with GCC; see the file COPYING3. If not see
#include "tree.h"
#include "tree-pass.h"
#include "cgraph.h"
+#include "tree-pretty-print.h"
+
+tree
+merge_comdat_group (tree *val2, tree newgroup,
+ symtab_node *symbol,
+ hash_map<tree, symtab_node *>& comdat_head_map)
+{
+ /* *val2 is TOP. */
+ if (val2 == NULL || *val2 == NULL)
+ return newgroup;
+
+ /* *val2 is BOTTOM. */
+ else if (*val2 == error_mark_node)
+ return error_mark_node;
+
+ /* *val2 is COMDAT. */
+ else
+ {
+ /* COMDAT meet TOP == COMDAT. */
+ if (newgroup == NULL)
+ return *val2;
+
+ /* COMDAT meet BOTTOM == BOTTOM. */
+ else if (newgroup == error_mark_node)
+ return error_mark_node;
+
+ /* COMDAT meet COMDAT == COMDAT if both are equal else
+ create new comdat group and assign it to symbol. */
+ else
+ {
+ if (newgroup == *val2)
+ return newgroup;
+
+ /* FIXME: using DECL_NAME (symbol->decl) as name of comdat section.
*/
+ newgroup = DECL_NAME (symbol->decl);
+ DECL_COMDAT (symbol->decl) = 1;
+ symbol->set_comdat_group (newgroup);
+ comdat_head_map.put (newgroup, symbol);
+ return newgroup;
+ }
+ }
+}
/* Main dataflow loop propagating comdat groups across
the symbol table. All references to SYMBOL are examined
@@ -63,7 +105,8 @@ 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_map<tree, symtab_node *>& comdat_head_map)
{
int i;
struct ipa_ref *ref;
@@ -78,7 +121,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,
comdat_head_map);
continue;
}
@@ -105,7 +148,8 @@ propagate_comdat_group (struct symtab_node *symbol,
/* The actual merge operation. */
tree *val2 = map.get (symbol2);
-
+ newgroup = merge_comdat_group (val2, newgroup, symbol, comdat_head_map);
+#if 0
if (val2 && *val2 != newgroup)
{
if (!newgroup)
@@ -113,6 +157,7 @@ propagate_comdat_group (struct symtab_node *symbol,
else
newgroup = error_mark_node;
}
+#endif
}
/* If we analyze function, walk also callers. */
@@ -129,7 +174,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,
comdat_head_map);
/* If we see inline clone, its comdat group actually
corresponds to the comdat group of the function it
is inlined to. */
@@ -140,7 +185,8 @@ propagate_comdat_group (struct symtab_node *symbol,
/* The actual merge operation. */
tree *val2 = map.get (symbol2);
-
+ newgroup = merge_comdat_group (val2, newgroup, symbol, comdat_head_map);
+#if 0
if (val2 && *val2 != newgroup)
{
if (!newgroup)
@@ -148,6 +194,7 @@ propagate_comdat_group (struct symtab_node *symbol,
else
newgroup = error_mark_node;
}
+#endif
}
return newgroup;
}
@@ -310,7 +357,7 @@ ipa_comdats (void)
if (group == error_mark_node)
continue;
- newgroup = propagate_comdat_group (symbol, group, map);
+ newgroup = propagate_comdat_group (symbol, group, map, comdat_head_map);
/* If nothing changed, proceed to next symbol. */
if (newgroup == group)
extern 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;
/* 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;
