[Bug target/63892] [5 Regression] gcc.dg/sibcall-3.c fails on darwin with -m32

["jakub at gcc dot gnu.org" <gcc-bugzilla at gcc dot gnu dot org>]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63892

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |hubicka at gcc dot gnu.org

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
So, during ICF we don't create alias because
sem_item::target_supports_symbol_aliases_p () tells us not to.
But I really don't see a reason why we couldn't try to redirect_callers in this
case, both original and alias are local functions without address taken.
There is another case that worries me - if the above mentioned function returns
false, we set create_thunk even for stdarg_p functions.

So, I think we should do something like:

--- gcc/ipa-icf.c.jj    2015-02-19 12:52:33.000000000 +0100
+++ gcc/ipa-icf.c    2015-02-20 10:06:04.532637240 +0100
@@ -651,7 +651,9 @@ sem_function::merge (sem_item *alias_ite
      section (or we risk link failures when section is discarded).  */
   if ((original_address_matters
        && alias_address_matters)
-      || original_discardable)
+      || original_discardable
+      || DECL_COMDAT_GROUP (alias->decl)
+      || !sem_item::target_supports_symbol_aliases_p ())
     {
       create_thunk = !stdarg_p (TREE_TYPE (alias->decl));
       create_alias = false;
@@ -659,6 +661,7 @@ sem_function::merge (sem_item *alias_ite
          the extra thunk wrapper for direct calls.  */
       redirect_callers
     = (!original_discardable
+       && !DECL_COMDAT_GROUP (alias->decl)
        && alias->get_availability () > AVAIL_INTERPOSABLE
        && original->get_availability () > AVAIL_INTERPOSABLE
        && !alias->instrumented_version);
@@ -670,13 +673,6 @@ sem_function::merge (sem_item *alias_ite
       redirect_callers = false;
     }

-  if (create_alias && (DECL_COMDAT_GROUP (alias->decl)
-               || !sem_item::target_supports_symbol_aliases_p ()))
-    {
-      create_alias = false;
-      create_thunk = true;
-    }
-
   /* We want thunk to always jump to the local function body
      unless the body is comdat and may be optimized out.  */
   if ((create_thunk || redirect_callers)

Unfortunately this ICEs on the testcase in question, when we reach:
725          /* The alias function is removed if symbol address
726             does not matter.  */
727          if (!alias_address_matters)
728        alias->remove ();
because in
1645    void
1646    sem_item_optimizer::remove_symtab_node (symtab_node *node)
1647    {
1648      gcc_assert (!m_classes.elements());
1649    
1650      m_removed_items_set.add (node);
1651    }
called during the alias->remove ()
m_classes.elements() is 3.  I'm not familiar with that code, Martin/Honza, can
you please have a look?

And, another thing, not analyzed fully, is that when we create a thunk to the
local calling convention function and the thunk isn't visible either, the
question is why it doesn't use the same calling convention (i.e. local again).

Last thing, I believe we should change the testcase anyway, either use
-fno-ipa-icf or add say some volatile int v; v++;
into one of the functions, so that sibcall-3.c tests what it originally meant
to test.  With ICF it can be turned into a tail recursion test, while clearly
originally it was meant to be a tail call test.
We can certainly copy the current testcase into a new one...