Re: [RFC PATCH] Implementing ifunc target hook

[Alexander Ivchenko <aivchenk at gmail dot com>]

Hi,

Since almost three months have passed I feel that I need to recheck the patch
before commiting it. I fixed what Maxim mentioned and also I fixed:

diff --git a/gcc/configure b/gcc/configure
old mode 100755
new mode 100644
index eac96cd..928693a
--- a/gcc/configure
+++ b/gcc/configure
@@ -22055,11 +22055,14 @@ else
   enable_gnu_indirect_function="$default_gnu_indirect_function"
 fi

-if test x$enable_gnu_indirect_function = xyes; then

-$as_echo "#define HAVE_GNU_INDIRECT_FUNCTION 1" >>confdefs.h
+gif=`if test $enable_gnu_indirect_function == yes; then echo 1; else
echo 0; fi`
+
+cat >>confdefs.h <<_ACEOF
+#define HAVE_GNU_INDIRECT_FUNCTION $gif
+_ACEOF
+

-fi

 if test $in_tree_ld != yes ; then
   ld_ver=`$gcc_cv_ld --version 2>/dev/null | sed 1q`
diff --git a/gcc/configure.ac b/gcc/configure.ac
index 40a1af7..51d334c 100644
--- a/gcc/configure.ac
+++ b/gcc/configure.ac
@@ -2299,10 +2299,11 @@ AC_ARG_ENABLE(gnu-indirect-function,
 Valid choices are 'yes' and 'no'.]) ;;
   esac],
  [enable_gnu_indirect_function="$default_gnu_indirect_function"])
-if test x$enable_gnu_indirect_function = xyes; then
-  AC_DEFINE(HAVE_GNU_INDIRECT_FUNCTION, 1,
-   [Define if your system supports gnu indirect functions.])
-fi
+
+gif=`if test $enable_gnu_indirect_function == yes; then echo 1; else
echo 0; fi`
+AC_DEFINE_UNQUOTED(HAVE_GNU_INDIRECT_FUNCTION, $gif,
+[Define if your system supports gnu indirect functions.])
+

HAVE_GNU_INDIRECT_FUNCTION was not defined on targets that don't have
the support of IFUNC
and the build of compiler could be broken. Now we define
HAVE_GNU_INDIRECT_FUNCTION as 0 in
those cases.

ok for trunk?

thanks,
Alexander

2013/1/15 Maxim Kuvyrkov <maxim.kuvyrkov@gmail.com>:
> On 15/01/2013, at 4:55 AM, Alexander Ivchenko wrote:
>
>> thank you very much for your review!
>>
>> I fixed the arm build and all other issues that you raised.
>>
>> the patch is attached. Bootstrap and tested on x86-64 linux
>
>
> The patch is OK with the cleanups mentioned below (no need to resubmit for review).  Unfortunately, you will have to wait for Stage 1 to commit your patch.
>
>
>> --- a/gcc/config.gcc
>> +++ b/gcc/config.gcc
>> @@ -636,6 +636,11 @@ case ${target} in
>>        native_system_header_dir=/include
>>        ;;
>>    esac
>> +  case $target in
>> +    *linux*)
>> +      tm_p_file="${tm_p_file} linux-protos.h"
>> +      ;;
>> +  esac
>>    # glibc / uclibc / bionic switch.
>>    # uclibc and bionic aren't usable for GNU/Hurd and neither for GNU/k*BSD.
>>    case $target in
>
> Can we merge this above hunk into subsequent "case $target" statement ...
>
>> @@ -661,8 +666,10 @@ case ${target} in
>>    # Add Android userspace support to Linux targets.
>>    case $target in
>>      *linux*)
>> +      tmake_file="${tmake_file} t-linux-android"
>>        tm_file="$tm_file linux-android.h"
>>        extra_options="$extra_options linux-android.opt"
>> +      extra_objs="$extra_objs linux-android.o"
>>        ;;
>>    esac
>
> ... here?
>
>>    # Enable compilation for Android by default for *android* targets.
>> @@ -863,7 +870,9 @@ arm*-*-netbsdelf*)
>>       tmake_file="${tmake_file} arm/t-arm"
>>       ;;
>>  arm*-*-linux-*)                      # ARM GNU/Linux with ELF
>> +     tmake_file="${tmake_file} t-linux-android"
>
> Merge this with tmake_file= setting a couple of lines down.  Put t-linux-android last on the line.
>
>>       tm_file="dbxelf.h elfos.h gnu-user.h linux.h linux-android.h glibc-stdint.h arm/elf.h arm/linux-gas.h arm/linux-elf.h"
>> +     extra_objs="$extra_objs linux-android.o"
>
> Please push extra_objs= setting a couple of lines down so that addition of t-linux-android and linux-android.o are side-by-side.
>
>>       case $target in
>>       arm*b-*-linux*)
>>           tm_defines="${tm_defines} TARGET_BIG_ENDIAN_DEFAULT=1"
>> diff --git a/gcc/config/linux-android.c b/gcc/config/linux-android.c
>> new file mode 100644
>> index 0000000..f3d82a5
>> --- /dev/null
>> +++ b/gcc/config/linux-android.c
>> @@ -0,0 +1,34 @@
>> +/* Functions for Linux Android as target machine for GNU C compiler.
>> +   Copyright (C) 2001, 2002, 2003, 2004, 2005, 2007, 2008, 2010, 2011,
>> +   2012, 2013.
>
> Should be "Copyright (C) 2013."  The copyright dates start with the year in which  a file was added.
>
> Also, for any file that your changes touch please add 2013 to the list of copyright years.  This is an annoying chore that committers have to do at the beginning of each year.
>
>> diff --git a/gcc/config/linux-protos.h b/gcc/config/linux-protos.h
>> new file mode 100644
>> index 0000000..aae1d28
>> --- /dev/null
>> +++ b/gcc/config/linux-protos.h
>> @@ -0,0 +1,22 @@
>> +/* Prototypes.
>> +   Copyright (C) 2001, 2002, 2003, 2004, 2005, 2007, 2008, 2010, 2011,
>> +   2012, 2013
>
> "Copyright (C) 2013."
>
>> --- /dev/null
>> +++ b/gcc/config/t-linux-android
>> @@ -0,0 +1,23 @@
>> +# Copyright (C) 2002, 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011, 2013
>
> "Copyright (C) 2013."
>
> Thanks!
>
> --
> Maxim Kuvyrkov
>

Attachment: disable_ifunc_for_android.patch
Description: Binary data