Re: [PATCH] Avoid an extra branch to PLT for -z now

[Rich Felker <dalias at libc dot org>]

On Wed, May 18, 2016 at 11:02:31AM -0700, H.J. Lu wrote:
> On Wed, May 4, 2016 at 11:17 AM, H.J. Lu <hongjiu.lu@intel.com> wrote:
> > When --enable-bind-now is used to configure glibc build, we can avoid
> > an extra branch to the PLT entry by using indirect branch via the GOT
> > slot instead, which is similar to the first instructuon in the PLT
> > entry.  Changes in the shared library sizes in text sections:
> >
> > Shared library    Before (bytes)   After (bytes)
> > libm.so             1060813          1060797
> > libmvec.so           160881           160805
> > libpthread.so         94992            94984
> > librt.so              25064            25048
> >
> > Tested on x86-64.  OK for master?
> >
> > H.J.
> > ---
> >         * config.h.in (BIND_NOW): New.
> >         * configure.ac (BIND_NOW): New.  Defined for --enable-bind-now.
> >         * configure: Regenerated.
> >         * sysdeps/x86_64/sysdep.h (JUMPTARGET): Defined to indirect
> >         branch via the GOT slot.
> > ---
> >  config.h.in             | 3 +++
> >  configure               | 4 ++++
> >  configure.ac            | 3 +++
> >  sysdeps/x86_64/sysdep.h | 8 ++++++--
> >  4 files changed, 16 insertions(+), 2 deletions(-)
> >
> > diff --git a/config.h.in b/config.h.in
> > index 0147ba3..2c902b0 100644
> > --- a/config.h.in
> > +++ b/config.h.in
> > @@ -87,6 +87,9 @@
> >     include/libc-symbols.h that avoid PLT slots in the shared objects.  */
> >  #undef NO_HIDDEN
> >
> > +/* Define this to disable lazy relocations in DSOs.  */
> > +#undef BIND_NOW
> > +
> >  /* AArch64 big endian ABI */
> >  #undef HAVE_AARCH64_BE
> >
> > diff --git a/configure b/configure
> > index 8fe5937..19a4829 100755
> > --- a/configure
> > +++ b/configure
> > @@ -3416,6 +3416,10 @@ else
> >  fi
> >
> >
> > +if test "x$bindnow" = xyes; then
> > +  $as_echo "#define BIND_NOW 1" >>confdefs.h
> > +
> > +fi
> >
> >  # Check whether --enable-static-nss was given.
> >  if test "${enable_static_nss+set}" = set; then :
> > diff --git a/configure.ac b/configure.ac
> > index 3c766b7..123f0d2 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -231,6 +231,9 @@ AC_ARG_ENABLE([bind-now],
> >               [bindnow=$enableval],
> >               [bindnow=no])
> >  AC_SUBST(bindnow)
> > +if test "x$bindnow" = xyes; then
> > +  AC_DEFINE(BIND_NOW)
> > +fi
> >
> >  dnl On some platforms we cannot use dynamic loading.  We must provide
> >  dnl static NSS modules.
> > diff --git a/sysdeps/x86_64/sysdep.h b/sysdeps/x86_64/sysdep.h
> > index fbe3560..75ac747 100644
> > --- a/sysdeps/x86_64/sysdep.h
> > +++ b/sysdeps/x86_64/sysdep.h
> > @@ -90,9 +90,13 @@ lose:                                                                              \
> >
> >  #undef JUMPTARGET
> >  #ifdef PIC
> > -#define JUMPTARGET(name)       name##@PLT
> > +# ifdef BIND_NOW
> > +#  define JUMPTARGET(name)     *name##@GOTPCREL(%rip)
> > +# else
> > +#  define JUMPTARGET(name)     name##@PLT
> > +# endif
> >  #else
> > -#define JUMPTARGET(name)       name
> > +# define JUMPTARGET(name)      name
> >  #endif
> >
> >  /* Local label name for asm code. */
> > --
> > 2.5.5
> >
> 
> Any objections?

Is there a reason you need to make it conditional? Just using
*name@GOTPCREL(%rip) unconditionally would force this particular
symbol to be resolved at load time, and doing that for the small
number of affected symbols is not going to make any noticable
difference to startup performance, but may noticably improve runtime
performance.

Rich