Re: [gofrontend-dev] [PATCH 4/4] Gccgo port to s390[x] -- part II
On Sat, Nov 15, 2014 at 07:46:40AM -0800, Ian Taylor wrote: On Thu, Nov 13, 2014 at 2:58 AM, Dominik Vogt v...@linux.vnet.ibm.com wrote: What do you think about the attached patches? They work for me, but I'm not sure whether the patch to go-test.exp is good because I know nothing about tcl. Looks plausible to me. All right, if you could take care of the two patches for (go-test.exp) I'll make a patch with the changes to the nilptr test for the Go repository. Ciao Dominik ^_^ ^_^ -- Dominik Vogt IBM Germany
Re: [gofrontend-dev] [PATCH 4/4] Gccgo port to s390[x] -- part II
On Thu, Nov 13, 2014 at 2:58 AM, Dominik Vogt v...@linux.vnet.ibm.com wrote: What do you think about the attached patches? They work for me, but I'm not sure whether the patch to go-test.exp is good because I know nothing about tcl. Looks plausible to me. Ian
Re: [gofrontend-dev] [PATCH 4/4] Gccgo port to s390[x] -- part II
On Fri, Nov 07, 2014 at 08:24:15AM -0800, Ian Taylor wrote: On Fri, Nov 7, 2014 at 12:51 AM, Dominik Vogt v...@linux.vnet.ibm.com wrote: On Thu, Nov 06, 2014 at 09:06:18AM -0800, Ian Taylor wrote: On Thu, Nov 6, 2014 at 4:04 AM, Dominik Vogt v...@linux.vnet.ibm.com wrote: On Tue, Nov 04, 2014 at 08:16:51PM -0800, Ian Taylor wrote: The way to do it is not by copying the test. If the test needs to be customized, add additional files that use // +build lines to pick which files is built. Move them into a directory, like method4.go or other tests that use rundir. Currently go-test.exp does not look at the build lines of the files in subdirectories. Before I add that to the gcc testsuite start adding that, is it certain that the golang testsuite will be able to understand that and compile only the requested files? Hmmm, that is a good point. The testsuite doesn't use the go command to build the files in subdirectories, so it won't honor the +build lines. I didn't think of that. Sorry for pointing you in the wrong direction. That's no problem, I can enhance go-test.exp in Gcc. The question is if test cases extended in such a way would run in the master Go repository too. Are the tests there run with the Go tool? What do you think about the attached patches? They work for me, but I'm not sure whether the patch to go-test.exp is good because I know nothing about tcl. Ciao Dominik ^_^ ^_^ -- Dominik Vogt IBM Germany From 155982afbcaf36fc79a89d222f0ff1b9da17464a Mon Sep 17 00:00:00 2001 From: Dominik Vogt v...@linux.vnet.ibm.com Date: Mon, 10 Nov 2014 13:21:42 +0100 Subject: [PATCH 1/6] go.test: Fix // +build lines that have only negative specifiers Lines like this did not cause the file to be built on other platforms: -- snip -- // +build !s390 !s390x -- snip -- --- gcc/testsuite/go.test/go-test.exp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/gcc/testsuite/go.test/go-test.exp b/gcc/testsuite/go.test/go-test.exp index 25e405b..590aaf9 100644 --- a/gcc/testsuite/go.test/go-test.exp +++ b/gcc/testsuite/go.test/go-test.exp @@ -469,6 +469,9 @@ proc go-gc-tests { } { set matches_pos 1 } elseif { [ regexp -line \[ \]windows\(\[ \]\|\$\) $test_line ] } { set matches_neg 1 + } elseif { [ regexp -line // \\+build(\[ \]+!\[^ !\]+)*\[ \]*\$ $test_line ] } { + # line has only negative terms, default to positive match + set matches_pos 1 } if { $matches_pos == 1 $matches_neg == 0 } { continue -- 1.8.4.2 gcc/testsuite/ChangeLog 2014-11-10 Dominik Vogt v...@linux.vnet.ibm.com * go.test/go-test.exp: Fix // +build lines that have only negative specifiers (prefixed with '!') From 35d0e1052dce2eabed57f3c8c76bfbb62cf48676 Mon Sep 17 00:00:00 2001 From: Dominik Vogt v...@linux.vnet.ibm.com Date: Tue, 4 Nov 2014 10:13:22 +0100 Subject: [PATCH 3/6] go.test: Do not run test nilptr.go on s390 - platform specific test. * Detects word boundaries to distinguish between s390 and s390x. (Switch to using regular expressions.) * Implement the Prefix '!' to exclude targets from build. --- gcc/testsuite/go.test/test/nilptr.go | 35 gcc/testsuite/go.test/test/nilptr_other.go | 31 + gcc/testsuite/go.test/test/nilptr_s390.go| 23 gcc/testsuite/go.test/test/nilptr_s390_common.go | 23 gcc/testsuite/go.test/test/nilptr_s390x.go | 23 5 files changed, 111 insertions(+), 24 deletions(-) create mode 100644 gcc/testsuite/go.test/test/nilptr_other.go create mode 100644 gcc/testsuite/go.test/test/nilptr_s390.go create mode 100644 gcc/testsuite/go.test/test/nilptr_s390_common.go create mode 100644 gcc/testsuite/go.test/test/nilptr_s390x.go diff --git a/gcc/testsuite/go.test/test/nilptr.go b/gcc/testsuite/go.test/test/nilptr.go index 9631d16..9bc8045 100644 --- a/gcc/testsuite/go.test/test/nilptr.go +++ b/gcc/testsuite/go.test/test/nilptr.go @@ -1,4 +1,4 @@ -// run +// skip // Copyright 2011 The Go Authors. All rights reserved. // Use of this source code is governed by a BSD-style @@ -11,22 +11,8 @@ package main import unsafe -// Having a big address space means that indexing -// at a 256 MB offset from a nil pointer might not -// cause a memory access fault. This test checks -// that Go is doing the correct explicit checks to catch -// these nil pointer accesses, not just relying on the hardware. -var dummy [256 20]byte // give us a big address space - func main() { - // the test only tests what we intend to test - // if dummy starts in the first 256 MB of memory. - // otherwise there might not be anything mapped - // at the address that might be accidentally - // dereferenced below. - if uintptr(unsafe.Pointer(dummy)) 25620 { - panic(dummy too far out) - } + sanityCheck() shouldPanic(p1) shouldPanic(p2) @@ -57,14 +43,15 @@ func shouldPanic(f
Re: [gofrontend-dev] [PATCH 4/4] Gccgo port to s390[x] -- part II
I'd still like to avoid the rampant duplication if possible. One approach would be to put most of the test in something like nilptr_tests.go marked with // skip. Then we can have top-level nilptrXX.go tests with +build lines that use // run nilptr_tests.go. I fail to see how that could be done with // run. There is one example use, namely cmplxdivide.go. That is not run in gcc because the run line does not match anything in go-test.exp. If I add a rule for that, how does that help me to compile a test that consists of multiple files? At the moment, I've no idea how to tackle the multi file problem with the existing go-test.exp. Ciao Dominik ^_^ ^_^ -- Dominik Vogt IBM Germany
Re: [gofrontend-dev] [PATCH 4/4] Gccgo port to s390[x] -- part II
On Mon, Nov 10, 2014 at 6:00 AM, Dominik Vogt v...@linux.vnet.ibm.com wrote: I'd still like to avoid the rampant duplication if possible. One approach would be to put most of the test in something like nilptr_tests.go marked with // skip. Then we can have top-level nilptrXX.go tests with +build lines that use // run nilptr_tests.go. I fail to see how that could be done with // run. There is one example use, namely cmplxdivide.go. That is not run in gcc because the run line does not match anything in go-test.exp. If I add a rule for that, how does that help me to compile a test that consists of multiple files? That test is run (all tests are run or explicitly skipped or marked unsupported). In go-test.exp look for $test_line == // run cmplxdivide1.go Ian
Re: [gofrontend-dev] [PATCH 4/4] Gccgo port to s390[x] -- part II
Ian Taylor i...@golang.org writes: I don't know what's up with the complex number change. In general the Go compiler and libraries go to some effort to produce the same answers on all platforms. We need to understand why we get different answers on s390 (you may understand the differences, but I don't). Oh, I know this one. I've even filed yet another bug about it: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=59714 I assume s390 has a fused multiply add instruction? It's because libgcc's implementation of complex division is written in a way such that gcc compiles an expression like a*b-c*d as fma(a,b,-c*d) and even if a==c and b==d the latter expression doesn't return 0 unless things are in power of 2 ratios with one another. I won't change the tests without a clear understanding of why we are changing them. I think the *real* fix is for libgcc to use Kahan's compensated algorithm for 2 by 2 determinants[1] to compute a*b-c*d when fma is available. Cheers, mwh [1] That's something like this: // This implements what is sometimes called Kahan's compensated algorithm for // 2 by 2 determinants. It returns a*b + c*d to a high degree of precision // but depends on a fused-multiply add operation that rounds once. // // The obvious algorithm has problems when a*b and c*d nearly cancel, but the // trick is the calculation of 'e': a*b = w + e is exact when the operands // are considered as real numbers. So if c*d nearly cancels out w, e restores // the result to accuracy. double Kahan(double a, double b, double c, double d) { double w, e, f; w = b * a; e = fma(b, a, -w); f = fma(d, c, w); return f + e; } Algorithms like this is why the fma operation was introduced in the first place! pgpEbbVCSQqC8.pgp Description: PGP signature
Re: [gofrontend-dev] [PATCH 4/4] Gccgo port to s390[x] -- part II
On Thu, Nov 06, 2014 at 09:06:18AM -0800, Ian Taylor wrote: On Thu, Nov 6, 2014 at 4:04 AM, Dominik Vogt v...@linux.vnet.ibm.com wrote: On Tue, Nov 04, 2014 at 08:16:51PM -0800, Ian Taylor wrote: The way to do it is not by copying the test. If the test needs to be customized, add additional files that use // +build lines to pick which files is built. Move them into a directory, like method4.go or other tests that use rundir. Currently go-test.exp does not look at the build lines of the files in subdirectories. Before I add that to the gcc testsuite start adding that, is it certain that the golang testsuite will be able to understand that and compile only the requested files? Hmmm, that is a good point. The testsuite doesn't use the go command to build the files in subdirectories, so it won't honor the +build lines. I didn't think of that. Sorry for pointing you in the wrong direction. That's no problem, I can enhance go-test.exp in Gcc. The question is if test cases extended in such a way would run in the master Go repository too. Are the tests there run with the Go tool? Ciao Dominik ^_^ ^_^ -- Dominik Vogt IBM Germany
Re: [gofrontend-dev] [PATCH 4/4] Gccgo port to s390[x] -- part II
On Fri, Nov 7, 2014 at 12:51 AM, Dominik Vogt v...@linux.vnet.ibm.com wrote: On Thu, Nov 06, 2014 at 09:06:18AM -0800, Ian Taylor wrote: On Thu, Nov 6, 2014 at 4:04 AM, Dominik Vogt v...@linux.vnet.ibm.com wrote: On Tue, Nov 04, 2014 at 08:16:51PM -0800, Ian Taylor wrote: The way to do it is not by copying the test. If the test needs to be customized, add additional files that use // +build lines to pick which files is built. Move them into a directory, like method4.go or other tests that use rundir. Currently go-test.exp does not look at the build lines of the files in subdirectories. Before I add that to the gcc testsuite start adding that, is it certain that the golang testsuite will be able to understand that and compile only the requested files? Hmmm, that is a good point. The testsuite doesn't use the go command to build the files in subdirectories, so it won't honor the +build lines. I didn't think of that. Sorry for pointing you in the wrong direction. That's no problem, I can enhance go-test.exp in Gcc. The question is if test cases extended in such a way would run in the master Go repository too. Are the tests there run with the Go tool? I'm sorry, I wasn't clear. The test cases will not work in the master Go repository. When I said the testsuite doesn't use go command I was referring to the master testsuite. Sorry for the confusion. Ian
Re: [gofrontend-dev] [PATCH 4/4] Gccgo port to s390[x] -- part II
On Tue, Nov 04, 2014 at 08:16:51PM -0800, Ian Taylor wrote: The way to do it is not by copying the test. If the test needs to be customized, add additional files that use // +build lines to pick which files is built. Move them into a directory, like method4.go or other tests that use rundir. Currently go-test.exp does not look at the build lines of the files in subdirectories. Before I add that to the gcc testsuite start adding that, is it certain that the golang testsuite will be able to understand that and compile only the requested files? Ciao Dominik ^_^ ^_^ -- Dominik Vogt IBM Germany
Re: [gofrontend-dev] [PATCH 4/4] Gccgo port to s390[x] -- part II
On Thu, Nov 6, 2014 at 4:04 AM, Dominik Vogt v...@linux.vnet.ibm.com wrote: On Tue, Nov 04, 2014 at 08:16:51PM -0800, Ian Taylor wrote: The way to do it is not by copying the test. If the test needs to be customized, add additional files that use // +build lines to pick which files is built. Move them into a directory, like method4.go or other tests that use rundir. Currently go-test.exp does not look at the build lines of the files in subdirectories. Before I add that to the gcc testsuite start adding that, is it certain that the golang testsuite will be able to understand that and compile only the requested files? Hmmm, that is a good point. The testsuite doesn't use the go command to build the files in subdirectories, so it won't honor the +build lines. I didn't think of that. Sorry for pointing you in the wrong direction. I'd still like to avoid the rampant duplication if possible. One approach would be to put most of the test in something like nilptr_tests.go marked with // skip. Then we can have top-level nilptrXX.go tests with +build lines that use // run nilptr_tests.go. (By the way, it's not golang; it's Go. Please try to avoid the term golang. Thanks.) Ian
Re: [gofrontend-dev] [PATCH 4/4] Gccgo port to s390[x] -- part II
On Tue, Nov 04, 2014 at 08:16:51PM -0800, Ian Taylor wrote: I committed the change to go-test.exp. Thanks. The other changes are not OK. As described in gcc/testsuite/go.test/test/README.gcc, the files in gcc/testsuite/go.test/test are an exact copy of the master Go testsuite. Any changes must be made to the master Go testsuite first. I understand that, but I'm unsure how to handle a set of patches that all depend on each other but refer to three different reposiories. So I posted this patch intentionally in the wrong place, not knowing how to do it in a better way. I don't know what's up with the complex number change. In general the Go compiler and libraries go to some effort to produce the same answers on all platforms. We need to understand why we get different answers on s390 (you may understand the differences, but I don't). I won't change the tests without a clear understanding of why we are changing them. It's actually not a Go specific problem, the same deviation occurs in C code too. The cause is that constant folding is done with a higher precision and may yield a different result than the run time calculations. There is a Gcc bug report for that issue: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60181 The nilptr test doesn't run on some other platforms when using gccgo--search for nilptr in go-test.exp. If you want to work out a way to change the master Go testsuite such that the nilptr test passes on more platforms, that would be great. I don't have the slightest clue how this could be done in a platform independent way because the test heavily depends on the target's memory map layout. The way to do it is not by copying the test. If the test needs to be customized, add additional files that use // +build lines to pick which files is built. Move them into a directory, like method4.go or other tests that use rundir. I'll check that. Ciao Dominik ^_^ ^_^ -- Dominik Vogt IBM Germany
Re: [gofrontend-dev] [PATCH 4/4] Gccgo port to s390[x] -- part II
On Wed, Nov 5, 2014 at 2:05 AM, Dominik Vogt v...@linux.vnet.ibm.com wrote: On Tue, Nov 04, 2014 at 08:16:51PM -0800, Ian Taylor wrote: I committed the change to go-test.exp. Thanks. The other changes are not OK. As described in gcc/testsuite/go.test/test/README.gcc, the files in gcc/testsuite/go.test/test are an exact copy of the master Go testsuite. Any changes must be made to the master Go testsuite first. I understand that, but I'm unsure how to handle a set of patches that all depend on each other but refer to three different reposiories. So I posted this patch intentionally in the wrong place, not knowing how to do it in a better way. Changes to the master Go repository must follow the procedure described at http://golang.org/doc/contribute.html. I don't know what's up with the complex number change. In general the Go compiler and libraries go to some effort to produce the same answers on all platforms. We need to understand why we get different answers on s390 (you may understand the differences, but I don't). I won't change the tests without a clear understanding of why we are changing them. It's actually not a Go specific problem, the same deviation occurs in C code too. The cause is that constant folding is done with a higher precision and may yield a different result than the run time calculations. There is a Gcc bug report for that issue: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60181 So far it doesn't sound appropriate to change the Go testsuite for this. If the immediate goal is simply to get the s390 tests to pass, let's change go-test.exp to xfail the test unless and until somebody figures out the whole issue. Ian
Re: [gofrontend-dev] [PATCH 4/4] Gccgo port to s390[x] -- part II
I committed the change to go-test.exp. Thanks. The other changes are not OK. As described in gcc/testsuite/go.test/test/README.gcc, the files in gcc/testsuite/go.test/test are an exact copy of the master Go testsuite. Any changes must be made to the master Go testsuite first. I don't know what's up with the complex number change. In general the Go compiler and libraries go to some effort to produce the same answers on all platforms. We need to understand why we get different answers on s390 (you may understand the differences, but I don't). I won't change the tests without a clear understanding of why we are changing them. The nilptr test doesn't run on some other platforms when using gccgo--search for nilptr in go-test.exp. If you want to work out a way to change the master Go testsuite such that the nilptr test passes on more platforms, that would be great. The way to do it is not by copying the test. If the test needs to be customized, add additional files that use // +build lines to pick which files is built. Move them into a directory, like method4.go or other tests that use rundir. Ian