[ipa-comdats] create a new comdat group for symbol if it's referenced from multiple comdat groups

[Prathamesh Kulkarni <prathamesh dot kulkarni at linaro dot org>]

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

Attachment: patch.diff
Description: Text document

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;