Re: [gofrontend-dev] [PATCH 4/4] Gccgo port to s390[x] -- part II

2014-11-17 Thread Dominik Vogt
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

2014-11-15 Thread Ian Taylor
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

2014-11-13 Thread Dominik Vogt
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

2014-11-10 Thread Dominik Vogt
 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

2014-11-10 Thread Ian Taylor
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

2014-11-09 Thread Michael Hudson-Doyle
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

2014-11-07 Thread Dominik Vogt
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

2014-11-07 Thread Ian Taylor
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

2014-11-06 Thread Dominik Vogt
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

2014-11-06 Thread Ian Taylor
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

2014-11-05 Thread Dominik Vogt
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

2014-11-05 Thread Ian Taylor
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

2014-11-04 Thread Ian Taylor
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