[PATCH] powerpc: Define USE_PPC64_NOTOC iff compiler and linker also supports it

[Adhemerval Zanella]

On 09/11/2021 14:44, Fangrui Song wrote:
> On 2021-11-09, Adhemerval Zanella wrote:
>>
>>
>> On 08/11/2021 19:48, Fangrui Song wrote:
>>> On 2021-11-08, Adhemerval Zanella wrote:
>>>> The @ntoc usage only yields any advantage on ISA 3.1+ machine (power10)
> 
> typo: @notoc
> 
> any => an ?

Ack.

> 
>>>> and for ld.bfd also when it see pcrel relocations used on the code
> 
> sees

Ack.

> 
>>>> (generated if compiler targets ISA 3.1+).  On bfd case ISA 3.1+
>>>> instruction on stubs are used iff linker also sees the new pc-relative
>>>> relocations (for instance R_PPC64_D34), otherwise it generates default
>>>> stubs (ppc64_elf_check_relocs:4700).
>>>>
>>>> This patch also help on linkers that do not implement this optimization,
>>>> since building for older ISA (such as 3.0 / power9) will also trigger
>>>> power10 stubs generation in the assembly code uses the NOTOC imacro.
>>>>
>>>> Checked on powerpc64le-linux-gnu.
>>>> ---
>>>> sysdeps/powerpc/powerpc64/configure    | 42 ++++++++++++++++----------
>>>> sysdeps/powerpc/powerpc64/configure.ac | 25 ++++++++++-----
>>>> 2 files changed, 43 insertions(+), 24 deletions(-)
>>>>
>>>> diff --git a/sysdeps/powerpc/powerpc64/configure b/sysdeps/powerpc/powerpc64/configure
>>>> index 5ce77af631..db18791a55 100644
>>>> --- a/sysdeps/powerpc/powerpc64/configure
>>>> +++ b/sysdeps/powerpc/powerpc64/configure
>>>> @@ -32,26 +32,36 @@ if test x$libc_cv_overlapping_opd = xyes; then
>>>>
>>>> fi
>>>>
>>>> -# @notoc started to be supported in GNU Binutils 2.31.
>>>> -
>>>> -{ $as_echo "$as_me:${as_lineno-$LINENO}: checking if the assembler supports @notoc" >&5
>>>> -$as_echo_n "checking if the assembler supports @notoc... " >&6; }
>>>> +# We check if compiler supports @notoc generation since there is no
>>>> +# gain by enabling it if it will be optimized away by the linker.
>>>> +# It also helps linkers that might not optimize it and end up
>>>> +# generating stubs with ISA 3.1 instruction even targetting older ISA.
>>>> +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking if the compiler supports @notoc" >&5
>>>> +$as_echo_n "checking if the compiler supports @notoc... " >&6; }
>>>> if ${libc_cv_ppc64_notoc+:} false; then :
>>>>   $as_echo_n "(cached) " >&6
>>>> else
>>>> -
>>>> -           cat confdefs.h - <<_ACEOF >conftest.$ac_ext
>>>> -/* end confdefs.h.  */
>>>> -
>>>> -void foo (void) {asm("b foo@notoc");}
>>>> -
>>>> -_ACEOF
>>>> -if ac_fn_c_try_compile "$LINENO"; then :
>>>> -  libc_cv_ppc64_notoc=yes
>>>> -else
>>>> +    cat > conftest.c <<EOF
>>>> +int bar (void);
>>>> +int foo (void) { return bar () + 1; }
>>>> +EOF
>>>>   libc_cv_ppc64_notoc=no
>>>> -fi
>>>> -rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
>>>> +  if { ac_try='${CC-cc} $CFLAGS $CPPFLAGS -S -o conftest.s conftest.c'
>>>> +  { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } >&5
>>>> +  (eval $ac_try) 2>&5
>>>> +  ac_status=$?
>>>> +  $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
>>>> +  test $ac_status = 0; }; } \
>>>> +     && { ac_try='grep -q -E 'bar@notoc' conftest.s'
>>>> +  { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } >&5
>>>> +  (eval $ac_try) 2>&5
>>>> +  ac_status=$?
>>>> +  $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
>>>> +  test $ac_status = 0; }; }
>>>> +  then
>>>> +    libc_cv_ppc64_notoc=yes
>>>> +  fi
>>>> +  rm -rf conftest.*
>>>> fi
>>>> { $as_echo "$as_me:${as_lineno-$LINENO}: result: $libc_cv_ppc64_notoc" >&5
>>>> $as_echo "$libc_cv_ppc64_notoc" >&6; }
>>>> diff --git a/sysdeps/powerpc/powerpc64/configure.ac b/sysdeps/powerpc/powerpc64/configure.ac
>>>> index b77156f696..05f0000807 100644
>>>> --- a/sysdeps/powerpc/powerpc64/configure.ac
>>>> +++ b/sysdeps/powerpc/powerpc64/configure.ac
>>>> @@ -22,13 +22,22 @@ if test x$libc_cv_overlapping_opd = xyes; then
>>>>   AC_DEFINE(USE_PPC64_OVERLAPPING_OPD)
>>>> fi
>>>>
>>>> -# @notoc started to be supported in GNU Binutils 2.31.
>>>> -AC_CACHE_CHECK([if the assembler supports @notoc],
>>>> -           libc_cv_ppc64_notoc, [
>>>> -           AC_COMPILE_IFELSE([AC_LANG_SOURCE([
>>>> -void foo (void) {asm("b foo@notoc");}
>>>> -          ])],
>>>> -          [libc_cv_ppc64_notoc=yes],
>>>> -          [libc_cv_ppc64_notoc=no])])
>>>> +# We check if compiler supports @notoc generation since there is no
>>>> +# gain by enabling it if it will be optimized away by the linker.
>>>> +# It also helps linkers that might not optimize it and end up
>>>> +# generating stubs with ISA 3.1 instruction even targetting older ISA.
>>>> +AC_CACHE_CHECK([if the compiler supports @notoc],
>>>> +           libc_cv_ppc64_notoc, [dnl
>>>> +  cat > conftest.c <<EOF
>>>> +int bar (void);
>>>> +int foo (void) { return bar () + 1; }
>>>> +EOF
>>>> +  libc_cv_ppc64_notoc=no
>>>> +  if AC_TRY_COMMAND([${CC-cc} $CFLAGS $CPPFLAGS -S -o conftest.s conftest.c]) \
>>>> +     && AC_TRY_COMMAND([grep -q -E 'bar@notoc' conftest.s])
>>>> +  then
>>>> +    libc_cv_ppc64_notoc=yes
>>>> +  fi
>>>> +  rm -rf conftest.*])
>>>> AS_IF([test x$libc_cv_ppc64_notoc = xyes],
>>>>       [AC_DEFINE(USE_PPC64_NOTOC)])
>>>> -- 
>>>> 2.32.0
>>>>
>>>
>>> I think the original fix to https://sourceware.org/bugzilla/show_bug.cgi?id=26173
>>> complicated things. A simpler fix is to just check whether CC CFLAGS
>>> generates @notoc and use that to guide whether assembly code needs @notoc.
>>
>> This is exactly what this patch does in fact: it check if $CC $CFLAGS
>> generates @notoc and defines USE_PPC64_NOTOC which is used internally
>> on NOTOC macro.
>>
>> The title is misleading in fact, is should be "powerpc: Define USE_PPC64_NOTOC
>> iff compiler supports it"
> 
> Thanks for the clarification. The new title looks good to me.
> 
> Alan has a question about an assembly usage, but that's orthogonal to
> the configure.ac fix.

I will let Tulio answer it because he was the one that actually pushed
for the change.  I haven't investigate it why we need to NOTOC on the
code, maybe we can simplify it.

> 
> Reviewed-by: Fangrui Song <maskray@google.com>