[PATCH] Makefile: set USE_PIC on Linux x86_64 for linking with Git.pm

[PATCH] Makefile: set USE_PIC on Linux x86_64 for linking with Git.pm

[Marco Roeland]

In commit 6294a10 it was noted that "on x86-64 it seems that Git.xs does
not link without compiling the main git objects with -fPIC". Set it
therefore automatically on this platform.

Signed-off-by: Marco Roeland <marco.roeland@xs4all.nl>
---
At the moment this is 'pu' stuff.
---
 Makefile |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/Makefile b/Makefile
index 2df5bd4..0f0e25a 100644
--- a/Makefile
+++ b/Makefile
@@ -254,6 +254,9 @@ # we had "elif" things would have been m
 
 ifeq ($(uname_S),Linux)
 	NO_STRLCPY = YesPlease
+	ifneq (,$(findstring x86_64,$(uname_M)))
+		USE_PIC = YesPlease
+	endif
 endif
 ifeq ($(uname_S),Darwin)
 	NEEDS_SSL_WITH_CRYPTO = YesPlease
-- 
1.4.1.rc1.g3550-dirty

Re: [PATCH] Makefile: set USE_PIC on Linux x86_64 for linking with Git.pm

[Junio C Hamano]

Marco Roeland <marco.roeland@xs4all.nl> writes:

> In commit 6294a10 it was noted that "on x86-64 it seems that Git.xs does
> not link without compiling the main git objects with -fPIC". Set it
> therefore automatically on this platform.

I agree with it in principle but was too lazy to do that myself.
I wonder it should be inside Linux, though.

>  ifeq ($(uname_S),Linux)
>  	NO_STRLCPY = YesPlease
> +	ifneq (,$(findstring x86_64,$(uname_M)))
> +		USE_PIC = YesPlease
> +	endif
>  endif

Re: [PATCH] Makefile: set USE_PIC on Linux x86_64 for linking with Git.pm

[Junio C Hamano]

Junio C Hamano <junkio@cox.net> writes:

> Marco Roeland <marco.roeland@xs4all.nl> writes:
>
>> In commit 6294a10 it was noted that "on x86-64 it seems that Git.xs does
>> not link without compiling the main git objects with -fPIC". Set it
>> therefore automatically on this platform.
>
> I agree with it in principle but was too lazy to do that myself.
> I wonder it should be inside Linux, though.
>
>>  ifeq ($(uname_S),Linux)
>>  	NO_STRLCPY = YesPlease
>> +	ifneq (,$(findstring x86_64,$(uname_M)))
>> +		USE_PIC = YesPlease
>> +	endif
>>  endif

In other words, I am wondering why you did not do this more
obvious one:

        ifeq ($(uname_M),x86_64)
                USE_PIC = YesPlease
        endif

My suspicion is that you protected that in Linux on purpose
because you know that my version would break for somebody else,
or because you are trying to be cautious not to break other
platforms you do not have access to, and I cannot tell which.

Re: [PATCH] Makefile: set USE_PIC on Linux x86_64 for linking with Git.pm

[Marco Roeland]

On Wednesday June 28th 2006 Junio C Hamano wrote:

> I agree with it in principle but was too lazy to do that myself.
> I wonder it should be inside Linux, though.
> 
> >  ifeq ($(uname_S),Linux)
> >  	NO_STRLCPY = YesPlease
> > +	ifneq (,$(findstring x86_64,$(uname_M)))
> > +		USE_PIC = YesPlease
> > +	endif
> >  endif

Yes, I wondered about that myself. I chose to be on the safe side: I
can and have tested this myself on Linux x86-64 but am not sure if it's
needed on the BSD's for example.

Even for Linux someone mentioned that probably i386 is the exception in
_not_ needing the -fPIC linkage. It might even be specific to the Perl
"xs" implementation specifics?

So I should have added "Works for me (TM)"! ;-)
-- 
Marco Roeland

Re: [PATCH] Makefile: set USE_PIC on Linux x86_64 for linking with Git.pm

[Marco Roeland]

On Wednesday June 28th 2006 Junio C Hamano wrote:

> >>  ifeq ($(uname_S),Linux)
> >>  	NO_STRLCPY = YesPlease
> >> +	ifneq (,$(findstring x86_64,$(uname_M)))
> >> +		USE_PIC = YesPlease
> >> +	endif
> >>  endif
> 
> In other words, I am wondering why you did not do this more
> obvious one:
> 
>         ifeq ($(uname_M),x86_64)
>                 USE_PIC = YesPlease
>         endif
> 
> My suspicion is that you protected that in Linux on purpose
> because you know that my version would break for somebody else,
> or because you are trying to be cautious not to break other
> platforms you do not have access to, and I cannot tell which.

Sorry for the confusion. Yes that construct is much more readable. I
copy and pasted it from another section in the Makefile and adapted it
to this use. I tested it and it worked so I decided no to change it
anymore. So that clears up the syntactical issue.

I certainly do not know cases outside Linux where this might break on
x86-64. I just tried to limit it to the case I could test. But perhaps
someone with an x86-64 BSD or Solaris might try it?

To paraphrase Dave Jones: I type 'make', it fails. Some 'git log' later
I realise I have to manually define 'USE_PIC'. Hey, why doesn't it work
automagically? Some 'git grep' and I spot a construct for specific
(sub)platforms. Monkey see, monkey do. I type 'make', it works and
monkey sends patch! Thats it! No subtleties involved;-)
-- 
Marco Roeland

[PATCH] Makefile: set USE_PIC on Linux x86_64 for linking with Git.xs

[Marco Roeland]

In commit 6294a10 it was noted that "on x86-64 it seems that Git.xs does
not link without compiling the main git objects with -fPIC". Set it
therefore automatically on this platform.

This patch does this only for _Linux_ x86-64, as that is the only x86-64
platform I have access to. But it might very well make sense on other
x86-64 platforms, please test and report if you have such a platform.

Signed-off-by: Marco Roeland <marco.roeland@xs4all.nl>
---
This applies to 'pu'. It is an amended version from an earlier one
with a simplification from Junio and a clarification why it is Linux
specific at the moment. The title was also slightly improved.
---
 Makefile |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/Makefile b/Makefile
index 2df5bd4..88cfe2b 100644
--- a/Makefile
+++ b/Makefile
@@ -254,6 +254,9 @@ # we had "elif" things would have been m
 
 ifeq ($(uname_S),Linux)
 	NO_STRLCPY = YesPlease
+	ifeq ($(uname_M),x86_64)
+		USE_PIC = YesPlease
+	endif
 endif
 ifeq ($(uname_S),Darwin)
 	NEEDS_SSL_WITH_CRYPTO = YesPlease
-- 
1.4.1.rc1.g3550-dirty

Re: [PATCH] Makefile: set USE_PIC on Linux x86_64 for linking with Git.pm

[Pavel Roskin]

On Wed, 2006-06-28 at 21:21 +0200, Marco Roeland wrote:
> I certainly do not know cases outside Linux where this might break on
> x86-64. I just tried to limit it to the case I could test. But perhaps
> someone with an x86-64 BSD or Solaris might try it?
> 
> To paraphrase Dave Jones: I type 'make', it fails. Some 'git log' later
> I realise I have to manually define 'USE_PIC'. Hey, why doesn't it work
> automagically?

Automagically?  You should search the archives for "Autoconf".  When I
proposed using it, the hell broke loose.  Now let me indulge in
Schadenfreude :-)

I guess I'll need to argue with a working patch next time.

-- 
Regards,
Pavel Roskin

Re: [PATCH] Makefile: set USE_PIC on Linux x86_64 for linking with Git.pm

[Junio C Hamano]

Marco Roeland <marco.roeland@xs4all.nl> writes:

> Even for Linux someone mentioned that probably i386 is the exception in
> _not_ needing the -fPIC linkage. It might even be specific to the Perl
> "xs" implementation specifics?

USE_PIC is for pleasing Perly git and nothing else right now.

> So I should have added "Works for me (TM)"! ;-)

That would have been more explicit way to tell me that this is a
partial solution and I should solicit help from people on other
platforms.

By the way, I had an impression that compiling things with -fPIC
when not necessary was generally a bad idea from performance
point of view.  If that is the case we might want to compile,
under USE_PIC, everything with -fPIC in a separate area to
compile and link with Git.xs, without affecting the C-only core
code.

I suspect this would largely depend on the architecture.  I ran
git-fsck-objects compiled with and without -fPIC (after "make
clean" to rebuild everything) on a fully packed copy of the
linux-2.6 repository on my x86_64 box, and did not see
meaningful differences:

: gitster; /usr/bin/time ../git.junio/git-fsck-objects-no-pic --full
109.71user 5.01system 1:54.89elapsed 99%CPU (0avgtext+0avgdata 0maxresident)k
0inputs+0outputs (14major+1834967minor)pagefaults 0swaps
: gitster; /usr/bin/time ../git.junio/git-fsck-objects-with-pic --full
109.05user 4.97system 1:54.08elapsed 99%CPU (0avgtext+0avgdata 0maxresident)k
0inputs+0outputs (0major+1834981minor)pagefaults 0swaps
: gitster;

Re: [PATCH] Makefile: set USE_PIC on Linux x86_64 for linking with Git.pm

[Sergey Vlasov]

[-- Attachment #1: Type: text/plain, Size: 2668 bytes --]

On Wed, 28 Jun 2006 14:24:37 -0700 Junio C Hamano wrote:

> Marco Roeland <marco.roeland@xs4all.nl> writes:
> 
> > Even for Linux someone mentioned that probably i386 is the exception in
> > _not_ needing the -fPIC linkage. It might even be specific to the Perl
> > "xs" implementation specifics?

In general, -fPIC is required when building shared libraries.  On some
systems (e.g., Linux/i386) you can get away without -fPIC, but with a
penalty on memory use and load time: non-PIC code will need relocations,
therefore its pages will no longer be shared between different
processes, and relocations will be performed immediately after loading
the shared library.

> USE_PIC is for pleasing Perly git and nothing else right now.
> 
> > So I should have added "Works for me (TM)"! ;-)
> 
> That would have been more explicit way to tell me that this is a
> partial solution and I should solicit help from people on other
> platforms.
> 
> By the way, I had an impression that compiling things with -fPIC
> when not necessary was generally a bad idea from performance
> point of view.  If that is the case we might want to compile,
> under USE_PIC, everything with -fPIC in a separate area to
> compile and link with Git.xs, without affecting the C-only core
> code.

This is exactly what libtool does (if both static and shared libraries
are compiled, each file is compiled twice - once with -fPIC -DPIC, and
once without these options).

But I suspect that even libtool won't help with Perl anyway, unless we
create a proper libgit.so and then link our Perl extension with it.

> I suspect this would largely depend on the architecture.  I ran
> git-fsck-objects compiled with and without -fPIC (after "make
> clean" to rebuild everything) on a fully packed copy of the
> linux-2.6 repository on my x86_64 box, and did not see
> meaningful differences:
> 
> : gitster; /usr/bin/time ../git.junio/git-fsck-objects-no-pic --full
> 109.71user 5.01system 1:54.89elapsed 99%CPU (0avgtext+0avgdata 0maxresident)k
> 0inputs+0outputs (14major+1834967minor)pagefaults 0swaps
> : gitster; /usr/bin/time ../git.junio/git-fsck-objects-with-pic --full
> 109.05user 4.97system 1:54.08elapsed 99%CPU (0avgtext+0avgdata 0maxresident)k
> 0inputs+0outputs (0major+1834981minor)pagefaults 0swaps
> : gitster;

This is because most of time is spent inside SHA-1 and zlib routines,
which are the same in these cases.  Using mozilla-sha1 code might show
some difference.

And the effect of -fPIC on x86_64 is smaller than on i386, because
x86_64 has 2x more registers than i386, therefore loss of one register
is less noticeable.

[-- Attachment #2: Type: application/pgp-signature, Size: 190 bytes --]

Re: [PATCH] Makefile: set USE_PIC on Linux x86_64 for linking with Git.pm

[Josef Weidendorfer]

On Thursday 29 June 2006 11:04, Sergey Vlasov wrote:
> And the effect of -fPIC on x86_64 is smaller than on i386, because
> x86_64 has 2x more registers than i386, therefore loss of one register
> is less noticeable.

According to the x86_64 ABI, x86_64 has no explicit GOT pointer.
Meaning: no additional register needed at all, as x86_64 has IP-relative
addressing. Thus, compiling with -fPIC on x86_64 probably has no
negative implications at all (?).

Josef

Re: [PATCH] Makefile: set USE_PIC on Linux x86_64 for linking with Git.pm

[Jakub Narebski]

Pavel Roskin <proski@gnu.org> wrote:

> On Wed, 2006-06-28 at 21:21 +0200, Marco Roeland wrote:
>> I certainly do not know cases outside Linux where this might break on
>> x86-64. I just tried to limit it to the case I could test. But perhaps
>> someone with an x86-64 BSD or Solaris might try it?
>> 
>> To paraphrase Dave Jones: I type 'make', it fails. Some 'git log' later
>> I realise I have to manually define 'USE_PIC'. Hey, why doesn't it work
>> automagically?
> 
> Automagically?  You should search the archives for "Autoconf".  When I
> proposed using it, the hell broke loose.  Now let me indulge in
> Schadenfreude :-)

If I remember correctly everybody agreed that autoconf is least evil of the
whole autotools package. pasky suggested to write ./configure script by
hand on #git...

I'm trying to do inobtrusive _optional_ autoconf support in the patch series
beginning with
  Message-ID: <200606290301.51657.jnareb@gmail.com>
  http://permalink.gmane.org/gmane.comp.version-control.git/22832

Please wait for the patch moving ./autoconf output away from config.mak
(as some people here requested), and do contribute! My autoconf/m4
experience is nonexistent (I'm learning it as I go). See comments in the
third [PATCH/RFC] in series.

BTW. patches are against master.

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

Re: [PATCH] Makefile: set USE_PIC on Linux x86_64 for linking with Git.pm

[Pavel Roskin]

Hi, Jakub!

I don't have the newsgroup access, so I'm copying to the mailing list
instead.

On Fri, 2006-06-30 at 00:22 +0200, Jakub Narebski wrote:

> If I remember correctly everybody agreed that autoconf is least evil of the
> whole autotools package.

Sort of.  Some people were less dismissive than others.

>  pasky suggested to write ./configure script by
> hand on #git...

That's probably not a good idea.  Autoconf can do this very well if used
correctly.  And I'm ready to help when it comes to correctness.

> I'm trying to do inobtrusive _optional_ autoconf support in the patch series
> beginning with
>   Message-ID: <200606290301.51657.jnareb@gmail.com>
>   http://permalink.gmane.org/gmane.comp.version-control.git/22832

The problem with optional support is that you suddenly have two
alternative mechanisms to adjust the build to the system, and both
should be kept in a  working condition.  But it's a good first step.

> Please wait for the patch moving ./autoconf output away from config.mak
> (as some people here requested), and do contribute! My autoconf/m4
> experience is nonexistent (I'm learning it as I go). See comments in the
> third [PATCH/RFC] in series.
> 
> BTW. patches are against master.

The link doesn't show the "@" characters correctly.  Maybe somebody
could establish a git repository?  Ideally, the autoconf changes should
go to one of the Git branches.

-- 
Regards,
Pavel Roskin

Re: [PATCH] Makefile: set USE_PIC on Linux x86_64 for linking with Git.pm

[Jakub Narebski]

Pavel Roskin write:
> I don't have the newsgroup access, so I'm copying to the mailing list
> instead.

Newsgroup should be tied to mailing list, so messages on newsgroup 
would appear on mailing list and vice versa. Patches were sent to 
mailing list.

>> I'm trying to do inobtrusive _optional_ autoconf support in the patch series
>> beginning with
>>   Message-ID: <200606290301.51657.jnareb@gmail.com>
>>   http://permalink.gmane.org/gmane.comp.version-control.git/22832
> 
> The problem with optional support is that you suddenly have two
> alternative mechanisms to adjust the build to the system, and both
> should be kept in a  working condition.  But it's a good first step.

The idea I started working with autoconf was to be able to say
%configure in .spec file. Then I thought that it would be nice
to have --without-expat, --with-curl=PATH etc. options. Then...

Additionally, optional support would be easier to accept I think.

[repeated]
>>   Message-ID: <200606290301.51657.jnareb@gmail.com>
>>   http://permalink.gmane.org/gmane.comp.version-control.git/22832
> 
> The link doesn't show the "@" characters correctly.  Maybe somebody
> could establish a git repository?  Ideally, the autoconf changes should
> go to one of the Git branches.

Try yet another git mailing list archive:
  http://www.gelato.unsw.edu.au/archives/git/0606/23225.html

I'll try to publish changes at
  http://front.fuw.edu.pl/jnareb/scm/git.git/
  http://front.fuw.edu.pl/cgi-bin/jnareb/gitweb.cgi?p=git.git
after I learn how to setup ssh proxy or equivalent...

-- 
Jakub Narebski
Poland