Re: libgo patch committed: Change build procedure to use build tags

[Ian Lance Taylor <iant@golang.org>]

On Fri, Aug 12, 2016 at 4:19 AM, Andreas Krebbel
<krebbel@linux.vnet.ibm.com> wrote:
> On 08/06/2016 02:36 AM, Ian Lance Taylor wrote:
>> Go packages use build tags (see the section on Build Constraints at
>> https://golang.org/pkg/go/build/) to select which files to build on
>> specific systems.
>>
>> Previously the libgo Makefile explicitly listed the set of files to
>> compile for each package.  For packages that use build tags, this
>> required a lot of awkward automake conditionals in the Makefile.
>>
>> This patch changes the build to look at the build tags in the files.
>> The new shell script libgo/match.sh does the matching.  This required
>> adjusting a lot of build tags, and removing some files that are never
>> used.  I verified that the exact same sets of files are compiled on
>> x86_64-pc-linux-gnu.  I also tested the build on i386-sun-solaris
>> (building for both 32-bit and 64-bit).
>>
>> Writing match.sh revealed some bugs in the build tag handling that
>> already exists, in a slightly different form, in the gotest shell
>> script.  This patch fixes those problems as well.
>>
>> The old code used automake conditionals to handle systems that were
>> missing strerror_r and wait4.  Rather than deal with those in Go,
>> those functions are now implemented in runtime/go-nosys.c when
>> necessary, so the Go code can simply assume that they exist.
>>
>> The os testsuite looked for dir_unix.go, which was never built for
>> gccgo and has now been removed.  I changed the testsuite to look for
>> dir.go instead.
>>
>> Note that if you have an existing build directory, you will have to
>> remove all the .dep files in TARGET/libgo after updating to this
>> patch.  There isn't anything that will force them to update
>> automatically.
>>
>> Bootstrapped on x86_64-pc-linux-gnu and i386-sun-solaris.  Ran Go
>> testsuite on x86_64-pc-linux-gnu.  Committed to mainline.
>>
>> Ian
>
> This appears to break libgo build on s390x:
>
> libtool: compile:  /home3/andreas/clean/gcc-7.0.0-64-clean-build/./gcc/gccgo
> -B/home3/andreas/clean/gcc-7.0.0-64-clean-build/./gcc/
> -B/home3/andreas/clean/gcc-7.0.0-64-clean-install/s390x-ibm-linux-gnu/bin/ -B/h
> ome3/andreas/clean/gcc-7.0.0-64-clean-install/s390x-ibm-linux-gnu/lib/ -isystem
> /home3/andreas/clean/gcc-7.0.0-64-clean-install/s390x-ibm-linux-gnu/include -isystem
> /home3/andreas/clean/gcc-7.0.0-64-clean-instal
> l/s390x-ibm-linux-gnu/sys-include -O2 -g -I . -c -fgo-pkgpath=hash/crc32
> /home3/andreas/clean/../gcc/libgo/go/hash/crc32/crc32.go
> /home3/andreas/clean/../gcc/libgo/go/hash/crc32/crc32_generic.go /home3/andreas/c
> lean/../gcc/libgo/go/hash/crc32/crc32_s390x.go  -fPIC -o hash/.libs/crc32.o
> /home3/andreas/clean/../gcc/libgo/go/hash/crc32/crc32_s390x.go:56:1: error: redefinition of
> ‘updateCastagnoli’
>  func updateCastagnoli(crc uint32, p []byte) uint32 {
>  ^
> /home3/andreas/clean/../gcc/libgo/go/hash/crc32/crc32_generic.go:12:1: note: previous definition of
> ‘updateCastagnoli’ was here
>  func updateCastagnoli(crc uint32, p []byte) uint32 {
>  ^
> /home3/andreas/clean/../gcc/libgo/go/hash/crc32/crc32_s390x.go:81:1: error: redefinition of ‘updateIEEE’
>  func updateIEEE(crc uint32, p []byte) uint32 {
>  ^
> /home3/andreas/clean/../gcc/libgo/go/hash/crc32/crc32_generic.go:20:1: note: previous definition of
> ‘updateIEEE’ was here
>  func updateIEEE(crc uint32, p []byte) uint32 {
>  ^
> make[4]: *** [hash/crc32.lo] Error 1
> make[4]: *** Waiting for unfinished jobs....
>
>
> Adding more "build ignore"s does fix the build for me.

Thanks for the patch.  Committed.   (FYI, you shouldn't make changes
directly to libgo in the GCC repository--the files there are copied
from a separate repository.  See libgo/README.)


> However, I'm wondering why we have to disable the asm tuned files for gccgo? For s390 I really would
> like to enable them since they make a huge difference.

I would like to enable them also.  They are not enabled today because
the assembler code is written in the syntax accepted by
https://golang.org/cmd/asm.  The files in question are

https://tip.golang.org/src/crypto/aes/asm_s390x.s
https://tip.golang.org/src/hash/crc32/crc32_s390x.s

If someone could rewrite those into gas syntax, we could use them in gccgo.


> The same is probably also required in ./hash/crc32/crc32_amd64p32.go
> I could add this as well.

It is not needed there, because GOARCH will never match amd64p32.
That is only used for NaCL, which gccgo does not support.

Ian