[PATCH roland/arm] ARM: Define PI_STATIC_AND_HIDDEN.

[PATCH roland/arm] ARM: Define PI_STATIC_AND_HIDDEN.

[Roland McGrath]

I suspect this is now safe.  (It caused no apparent mischief in my
arm-linux-gnueabihf build using GCC 4.8.2.)  But I don't actually know
what GCC version is implied by "added by the GCC TLS patches".  I'm
guessing that 4.6 (our minimum) is new enough that this is always true
now.  If that's true, it would be nice to know the appropriate GCC
version number for sure and mention that in the comment.  If it is
really still the case that some supported compiler versions or
configurations do not reliably do pure PI access to static/hidden,
then I'd like to have enough information about that to write a proper
configure test.


Thanks,
Roland


2015-04-14  Roland McGrath  <roland@hack.frob.com>

	* sysdeps/arm/configure.ac (PI_STATIC_AND_HIDDEN): Define it.
	* sysdeps/arm/configure: Regenerated.

--- a/sysdeps/arm/configure
+++ b/sysdeps/arm/configure
@@ -1,7 +1,8 @@
 # This file is generated from configure.ac by Autoconf.  DO NOT EDIT!
  # Local configure fragment for sysdeps/arm.
 
-#AC_DEFINE(PI_STATIC_AND_HIDDEN)
+$as_echo "#define PI_STATIC_AND_HIDDEN 1" >>confdefs.h
+
 
 # We check to see if the compiler and flags are
 # selecting the hard-float ABI and if they are then
--- a/sysdeps/arm/configure.ac
+++ b/sysdeps/arm/configure.ac
@@ -3,9 +3,7 @@ GLIBC_PROVIDES dnl See aclocal.m4 in the top level source directory.
 
 dnl It is always possible to access static and hidden symbols in an
 dnl position independent way.
-dnl NOTE: This feature was added by the GCC TLS patches.  We should test for
-dnl it.  Until we do, don't define it.
-#AC_DEFINE(PI_STATIC_AND_HIDDEN)
+AC_DEFINE(PI_STATIC_AND_HIDDEN)
 
 # We check to see if the compiler and flags are
 # selecting the hard-float ABI and if they are then

Re: [PATCH roland/arm] ARM: Define PI_STATIC_AND_HIDDEN.

[Chris Metcalf]

On 04/14/2015 06:39 PM, Roland McGrath wrote:
> I suspect this is now safe.  (It caused no apparent mischief in my
> arm-linux-gnueabihf build using GCC 4.8.2.)  But I don't actually know
> what GCC version is implied by "added by the GCC TLS patches".  I'm
> guessing that 4.6 (our minimum) is new enough that this is always true
> now.  If that's true, it would be nice to know the appropriate GCC
> version number for sure and mention that in the comment.  If it is
> really still the case that some supported compiler versions or
> configurations do not reliably do pure PI access to static/hidden,
> then I'd like to have enough information about that to write a proper
> configure test.

Did you do any benchmarking?  I enabled this on tilegx out of curiosity,
and I found it was both larger (ld.so size increased 0.3%) and slower
(average of 0.1% slower when running fork/exec of a no-op program
in a loop with 10,000 iterations).  The timing results do vary enough
that I'd want to do more extensive testing to be able to say anything
definitive, but it's not very encouraging.

I admit I'm not sure why this might be, but it seems like the right
things were happening, e.g. _dl_start_final is missing and _dl_start
is bigger in the version with PI_STATIC_AND_HIDDEN defined.

All that said, if it's the more standard way, or is desirable for some
other reason, I'm happy to enable it for tilegx, but...

-- 
Chris Metcalf, EZChip Semiconductor
http://www.ezchip.com

Re: [PATCH roland/arm] ARM: Define PI_STATIC_AND_HIDDEN.

[Roland McGrath]

> Did you do any benchmarking?  I enabled this on tilegx out of curiosity,
> and I found it was both larger (ld.so size increased 0.3%) and slower
> (average of 0.1% slower when running fork/exec of a no-op program
> in a loop with 10,000 iterations).  The timing results do vary enough
> that I'd want to do more extensive testing to be able to say anything
> definitive, but it's not very encouraging.

I did not.  Just now I've looked at the sizes on arm-linux-gnueabihf
(-mthumb), and indeed this increases ld.so's text by 256 bytes (to
93989 from 94245).  For -marm the difference is only 192 bytes (to
126581 from 126389).

> I admit I'm not sure why this might be, but it seems like the right
> things were happening, e.g. _dl_start_final is missing and _dl_start
> is bigger in the version with PI_STATIC_AND_HIDDEN defined.

My guess is that there is some extra arithmetic to compute runtime
addresses from PC-relative ones.  Conversely, without
PI_STATIC_AND_HIDDEN defined the equivalent accesses are SP-relative
(and thus simpler) while the extra code to copy fields from the stack
struct to the hidden global struct is less than that increase.

But I didn't actually compare the code even for ARM, let alone Tile.

> All that said, if it's the more standard way, or is desirable for some
> other reason, I'm happy to enable it for tilegx, but...

It's certainly the more standard way in the sense that most machines,
including the most-used machines (x86), do it that way.  The recent
MIPS bug is an indication that the stack bootstrap_map code path is
less tested and perhaps less generally reliable.  I'd say that the
PI_STATIC_AND_HIDDEN case is just more straightforward and clean, so
if we can eventually get every machine into that state, that would be
ideal.

For x86_64, PC-relative accesses are entirely free.  (Well, you forego
indexed addressing modes you might sometimes use with absolute or
SP-relative accesses, but there is no PC-relative overhead per se.)
Not surprisingly, turning PI_STATIC_AND_HIDDEN off there adds 256
bytes to ld.so.  (Actually it is slightly surprising that it's exactly
+256 while ARM/Thumb is exactly -256. :-) It's just entirely expected
that it's a loser on x86_64.)

For i386, PC-relative accesses are relatively costly.  So it's a mild
surprise that turning PI_STATIC_AND_HIDDEN off there adds 188 bytes
(i.e. fewer PC-relative accesses needs more code, perhaps mostly the
extra copy-from-stack code).  My guess is that the compiler backend
for i386 has had more effort put into optimizing these cases.  For
example, perhaps it computes the common base address only once in the
function and reuses it more effectively while the ARM and TIle
backends are repeating the computation more often.  Given that i386
has more register pressure than any other machine, the fact that they
can win here suggests that you could too with sufficient work on the
compiler side.  (But this is all just guesses.)

(All these examples were with GCC 4.8.2 as modified by Ubuntu.
For arm-nacl with GCC 4.9.2 as modified by me, it adds 512 bytes.)


Thanks,
Roland

[COMMITTED] tile: Enable PI_STATIC_AND_HIDDEN

[Chris Metcalf]

This does make ld.so very slightly larger (0.3%) and doesn't seem to
actually improve performance; in fact, my limited testing suggested a
slight (0.1%) performance decrease (running fork/exec of a no-op program
in a loop), but I didn't do enough testing to establish statistical
significance.

However, Roland agrees that it makes sense to switch tile to using
this path, since it's the more standard way.
---
 ChangeLog                 | 5 +++++
 sysdeps/tile/configure    | 7 +++++++
 sysdeps/tile/configure.ac | 6 ++++++
 3 files changed, 18 insertions(+)
 create mode 100644 sysdeps/tile/configure
 create mode 100644 sysdeps/tile/configure.ac

diff --git a/ChangeLog b/ChangeLog
index 8f7c17603c9d..344c575192d3 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+2015-04-16  Chris Metcalf  <cmetcalf@ezchip.com>
+
+	* sysdeps/tile/configure.ac: New file.
+	* sysdeps/tile/configure: Regenerated.
+
 2015-04-15  Adhemerval Zanella  <adhemerval.zanella@linaro.org>
 
 	* wcsmbs/wcschr.c [WCSCHR] (wcschr): Define as __wcschr.  Remove
diff --git a/sysdeps/tile/configure b/sysdeps/tile/configure
new file mode 100644
index 000000000000..534c7dbc5d22
--- /dev/null
+++ b/sysdeps/tile/configure
@@ -0,0 +1,7 @@
+# This file is generated from configure.ac by Autoconf.  DO NOT EDIT!
+ # Local configure fragment for sysdeps/tile.
+
+# We can always access static and hidden symbols in a position independent way.
+$as_echo "#define PI_STATIC_AND_HIDDEN 1" >>confdefs.h
+
+# work around problem with autoconf and empty lines at the end of files
diff --git a/sysdeps/tile/configure.ac b/sysdeps/tile/configure.ac
new file mode 100644
index 000000000000..6e246a3bc927
--- /dev/null
+++ b/sysdeps/tile/configure.ac
@@ -0,0 +1,6 @@
+GLIBC_PROVIDES dnl See aclocal.m4 in the top level source directory.
+# Local configure fragment for sysdeps/tile.
+
+# We can always access static and hidden symbols in a position independent way.
+AC_DEFINE(PI_STATIC_AND_HIDDEN)
+# work around problem with autoconf and empty lines at the end of files
-- 
2.1.2

Re: [PATCH roland/arm] ARM: Define PI_STATIC_AND_HIDDEN.

[Joseph Myers]

On Tue, 14 Apr 2015, Roland McGrath wrote:

> I suspect this is now safe.  (It caused no apparent mischief in my
> arm-linux-gnueabihf build using GCC 4.8.2.)  But I don't actually know
> what GCC version is implied by "added by the GCC TLS patches".  I'm

I believe it means 4.1.

> 2015-04-14  Roland McGrath  <roland@hack.frob.com>
> 
> 	* sysdeps/arm/configure.ac (PI_STATIC_AND_HIDDEN): Define it.
> 	* sysdeps/arm/configure: Regenerated.

OK.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH roland/arm] ARM: Define PI_STATIC_AND_HIDDEN.

[Roland McGrath]

Committed with improved comment.

Thanks,
Roland