Joseph, Maxim, thank you for your input. I converted this macro into
a target hook as you said. I had to add gcc/config/linux-protos.h in order
to declare linux (that works for android) version of this hook - otherwise
I don't know where to declare it..

>> --- a/gcc/config/i386/i386.c
>> +++ b/gcc/config/i386/i386.c
>> @@ -29146,7 +29146,7 @@ make_name (tree decl, const char *suffix, bool make_unique)
>>    return global_var_name;
>>  }
>>
>> -#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) && HAVE_GNU_INDIRECT_FUNCTION
>> +#if defined (ASM_OUTPUT_TYPE_DIRECTIVE)
>>
>>  /* Make a dispatcher declaration for the multi-versioned function DECL.
>>     Calls to DECL function will be replaced with calls to the dispatcher
>> @@ -29213,7 +29213,7 @@ ix86_get_function_versions_dispatcher (void *decl)
>>
>>    tree dispatch_decl = NULL;
>>
>> -#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) && HAVE_GNU_INDIRECT_FUNCTION
>> +#if defined (ASM_OUTPUT_TYPE_DIRECTIVE)
>>    struct cgraph_function_version_info *it_v = NULL;
>>    struct cgraph_node *dispatcher_node = NULL;
>>    struct cgraph_function_version_info *dispatcher_version_info = NULL;
>
>It seems you can move these variables inside the 'if (TARGET_HAS_IFUNC)' clause below and make >the code cleaner, no?

All variable declarations should be at the beginning of the routine.
Or is it changed right now?

>> @@ -29263,24 +29263,33 @@ ix86_get_function_versions_dispatcher (void *decl)
>>
>>    default_node = default_version_info->this_node;
>>
>> -#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) && HAVE_GNU_INDIRECT_FUNCTION
>> -  /* Right now, the dispatching is done via ifunc.  */
>> -  dispatch_decl = make_dispatcher_decl (default_node->symbol.decl);
>> -
>> -  dispatcher_node = cgraph_get_create_node (dispatch_decl);
>> -  gcc_assert (dispatcher_node != NULL);
>> -  dispatcher_node->dispatcher_function = 1;
>> -  dispatcher_version_info
>> -    = insert_new_cgraph_node_version (dispatcher_node);
>> -  dispatcher_version_info->next = default_version_info;
>> -  dispatcher_node->local.finalized = 1;
>> -
>> -  /* Set the dispatcher for all the versions.  */
>> -  it_v = default_version_info;
>> -  while (it_v->next != NULL)
>> +#if defined (ASM_OUTPUT_TYPE_DIRECTIVE)
>> +  if (TARGET_HAS_IFUNC)
>> +    {
>> +      /* Right now, the dispatching is done via ifunc.  */
>> +      dispatch_decl = make_dispatcher_decl (default_node->symbol.decl);
>> +
>> +      dispatcher_node = cgraph_get_create_node (dispatch_decl);
>> +      gcc_assert (dispatcher_node != NULL);
>> +      dispatcher_node->dispatcher_function = 1;
>> +      dispatcher_version_info
>> +     = insert_new_cgraph_node_version (dispatcher_node);
>> +      dispatcher_version_info->next = default_version_info;
>> +      dispatcher_node->local.finalized = 1;
>> +
>> +      /* Set the dispatcher for all the versions.  */
>> +      it_v = default_version_info;
>> +      while (it_v->next != NULL)
>> +     {
>> +       it_v->dispatcher_resolver = dispatch_decl;
>> +       it_v = it_v->next;
>> +     }
>> +    }
>> +  else
>>      {
>> -      it_v->dispatcher_resolver = dispatch_decl;
>> -      it_v = it_v->next;
>> +      error_at (DECL_SOURCE_LOCATION (default_node->symbol.decl),
>> +             "multiversioning needs ifunc which is not supported "
>> +             "on this target");
>>      }
>
>This looks wrong.  Before the patch this code would be ignored if !HAVE_GNU_INDIRECT_FUNCTION and after the patch it will produce an error.  Removing the else-clause should fix this.

Mmm, I believe that we want this code not to be ignored, because we
want it to produces an error
if the target does not support ifuncs and we are compiling something
that uses ifuncs.. May be I
missed something..

>Please use same error message in both error cases.  It seems you can just convert '#else' >preprocessor clause into 'else' code clause without touching the error_at() statement beyond >indentation updates.
Done.

thank you,
Alexander

2012/12/28 Maxim Kuvyrkov <maxim@codesourcery.com>:
> On 27/12/2012, at 1:15 AM, Alexander Ivchenko wrote:
>
>> Hi,
>>
>> Currently Android dynamic loader does not support indirect functions
>> (And I don't think that
>> it will someday). But there is no way for us to specify that for gcc,
>> and for example, tests like
>> gcc.dg/attr-ifunc-* are failing on android right now.
>> The attached patch is indended to add the target hook for indicating
>> the support of ifunc on target.
>
> The idea behind the patch looks OK, but implementation needs a bit of tweaking.
>
> As Joseph mentioned, you need to convert this macro into a target hook.  GCC is making a gradual transition away from target macros to target hook functions, and all new hooks should be added as functions, unless there is a compelling argument to make it a macro.  For an example of adding a target hook function see rev 194608 (among many others).
>
>> --- a/gcc/config/i386/i386.c
>> +++ b/gcc/config/i386/i386.c
>> @@ -29146,7 +29146,7 @@ make_name (tree decl, const char *suffix, bool make_unique)
>>    return global_var_name;
>>  }
>>
>> -#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) && HAVE_GNU_INDIRECT_FUNCTION
>> +#if defined (ASM_OUTPUT_TYPE_DIRECTIVE)
>>
>>  /* Make a dispatcher declaration for the multi-versioned function DECL.
>>     Calls to DECL function will be replaced with calls to the dispatcher
>> @@ -29213,7 +29213,7 @@ ix86_get_function_versions_dispatcher (void *decl)
>>
>>    tree dispatch_decl = NULL;
>>
>> -#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) && HAVE_GNU_INDIRECT_FUNCTION
>> +#if defined (ASM_OUTPUT_TYPE_DIRECTIVE)
>>    struct cgraph_function_version_info *it_v = NULL;
>>    struct cgraph_node *dispatcher_node = NULL;
>>    struct cgraph_function_version_info *dispatcher_version_info = NULL;
>
> It seems you can move these variables inside the 'if (TARGET_HAS_IFUNC)' clause below and make the code cleaner, no?
>
>> @@ -29263,24 +29263,33 @@ ix86_get_function_versions_dispatcher (void *decl)
>>
>>    default_node = default_version_info->this_node;
>>
>> -#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) && HAVE_GNU_INDIRECT_FUNCTION
>> -  /* Right now, the dispatching is done via ifunc.  */
>> -  dispatch_decl = make_dispatcher_decl (default_node->symbol.decl);
>> -
>> -  dispatcher_node = cgraph_get_create_node (dispatch_decl);
>> -  gcc_assert (dispatcher_node != NULL);
>> -  dispatcher_node->dispatcher_function = 1;
>> -  dispatcher_version_info
>> -    = insert_new_cgraph_node_version (dispatcher_node);
>> -  dispatcher_version_info->next = default_version_info;
>> -  dispatcher_node->local.finalized = 1;
>> -
>> -  /* Set the dispatcher for all the versions.  */
>> -  it_v = default_version_info;
>> -  while (it_v->next != NULL)
>> +#if defined (ASM_OUTPUT_TYPE_DIRECTIVE)
>> +  if (TARGET_HAS_IFUNC)
>> +    {
>> +      /* Right now, the dispatching is done via ifunc.  */
>> +      dispatch_decl = make_dispatcher_decl (default_node->symbol.decl);
>> +
>> +      dispatcher_node = cgraph_get_create_node (dispatch_decl);
>> +      gcc_assert (dispatcher_node != NULL);
>> +      dispatcher_node->dispatcher_function = 1;
>> +      dispatcher_version_info
>> +     = insert_new_cgraph_node_version (dispatcher_node);
>> +      dispatcher_version_info->next = default_version_info;
>> +      dispatcher_node->local.finalized = 1;
>> +
>> +      /* Set the dispatcher for all the versions.  */
>> +      it_v = default_version_info;
>> +      while (it_v->next != NULL)
>> +     {
>> +       it_v->dispatcher_resolver = dispatch_decl;
>> +       it_v = it_v->next;
>> +     }
>> +    }
>> +  else
>>      {
>> -      it_v->dispatcher_resolver = dispatch_decl;
>> -      it_v = it_v->next;
>> +      error_at (DECL_SOURCE_LOCATION (default_node->symbol.decl),
>> +             "multiversioning needs ifunc which is not supported "
>> +             "on this target");
>>      }
>
> This looks wrong.  Before the patch this code would be ignored if !HAVE_GNU_INDIRECT_FUNCTION and after the patch it will produce an error.  Removing the else-clause should fix this.
>
>>  #else
>>    error_at (DECL_SOURCE_LOCATION (default_node->symbol.decl),
>> diff --git a/gcc/config/linux-android.h b/gcc/config/linux-android.h
>> index e74e261..f6f44f1 100644
>> --- a/gcc/config/linux-android.h
>> +++ b/gcc/config/linux-android.h
>> @@ -58,3 +58,6 @@
>>
>>  #define ANDROID_ENDFILE_SPEC \
>>    "%{shared: crtend_so%O%s;: crtend_android%O%s}"
>> +
>> +#undef TARGET_HAS_IFUNC
>> +#define TARGET_HAS_IFUNC (!TARGET_ANDROID)
>
> This initialization should be moved to targethooks.c ...
>
>> diff --git a/gcc/defaults.h b/gcc/defaults.h
>> index 76909ab..2180a47 100644
>> --- a/gcc/defaults.h
>> +++ b/gcc/defaults.h
>> @@ -111,6 +111,10 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
>>  #endif
>>  #endif
>>
>> +#ifndef TARGET_HAS_IFUNC
>> +#define TARGET_HAS_IFUNC HAVE_GNU_INDIRECT_FUNCTION
>> +#endif
>
> ... and this one too.
>
>> +
>>  #ifndef IFUNC_ASM_TYPE
>>  #define IFUNC_ASM_TYPE "gnu_indirect_function"
>>  #endif
>> diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
>> index 75aa867..1c8bc51 100644
>> --- a/gcc/doc/tm.texi
>> +++ b/gcc/doc/tm.texi
>> @@ -5847,6 +5847,12 @@ If @code{ASM_OUTPUT_DEF} is not available, the hook's default definition
>>  is @code{NULL}, which disables the use of section anchors altogether.
>>  @end deftypefn
>>
>> +@defmac TARGET_HAS_IFUNC
>> +When this macro is nonzero, GCC will assume that target supports GNU indirect
>> +functions.  The support includes the assembler, linker and dynamic linker. The
>> +default value of this hook is defined as for the host machine.
>> +@end defmac
>> +
>>  @deftypefn {Target Hook} bool TARGET_USE_ANCHORS_FOR_SYMBOL_P (const_rtx @var{x})
>>  Return true if GCC should attempt to use anchors to access @code{SYMBOL_REF}
>>  @var{x}.  You can assume @samp{SYMBOL_REF_HAS_BLOCK_INFO_P (@var{x})} and
>> diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in
>> index 95fab18..e1646b8 100644
>> --- a/gcc/doc/tm.texi.in
>> +++ b/gcc/doc/tm.texi.in
>> @@ -5751,6 +5751,12 @@ If @code{ASM_OUTPUT_DEF} is not available, the hook's default definition
>>  is @code{NULL}, which disables the use of section anchors altogether.
>>  @end deftypefn
>>
>> +@defmac TARGET_HAS_IFUNC
>> +When this macro is nonzero, GCC will assume that target supports GNU indirect
>> +functions.  The support includes the assembler, linker and dynamic linker. The
>> +default value of this hook is defined as for the host machine.
>> +@end defmac
>> +
>>  @hook TARGET_USE_ANCHORS_FOR_SYMBOL_P
>>  Return true if GCC should attempt to use anchors to access @code{SYMBOL_REF}
>>  @var{x}.  You can assume @samp{SYMBOL_REF_HAS_BLOCK_INFO_P (@var{x})} and
>> diff --git a/gcc/varasm.c b/gcc/varasm.c
>> index 7d083fd..14be8fb 100644
>> --- a/gcc/varasm.c
>> +++ b/gcc/varasm.c
>> @@ -5489,10 +5489,14 @@ do_assemble_alias (tree decl, tree target)
>>      }
>>    if (lookup_attribute ("ifunc", DECL_ATTRIBUTES (decl)))
>>      {
>> -#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) && HAVE_GNU_INDIRECT_FUNCTION
>> -      ASM_OUTPUT_TYPE_DIRECTIVE
>> -     (asm_out_file, IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (decl)),
>> -      IFUNC_ASM_TYPE);
>> +#if defined (ASM_OUTPUT_TYPE_DIRECTIVE)
>> +      if (TARGET_HAS_IFUNC)
>> +     ASM_OUTPUT_TYPE_DIRECTIVE
>> +       (asm_out_file, IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (decl)),
>> +        IFUNC_ASM_TYPE);
>> +      else
>> +     error_at (DECL_SOURCE_LOCATION (decl),
>> +               "ifunc is not supported on this target");
>>  #else
>>        error_at (DECL_SOURCE_LOCATION (decl),
>>               "ifunc is not supported in this configuration");
>
> Please use same error message in both error cases.  It seems you can just convert '#else' preprocessor clause into 'else' code clause without touching the error_at() statement beyond indentation updates.
>
> Thanks,
>
> --
> Maxim Kuvyrkov
> CodeSourcery / Mentor Graphics
>