[PATCH 0/4] Gccgo port to s390[x] -- part II

[Dominik Vogt]

This is the second set of patches to support s390[x] with Gccgo,
containing mostly architecture dependent parts that affect the
following directories:

gcc/testsuite/go.test (golang)
gcc/testsuite/go.test/go-test.exp (Gcc?)
libgo (Gcc?)
libgo/go (gofrontend)
libgo/runtime (gofrontend)

It seemed infeasible to split this patch series and send the
patches only to the "right" places.

All patches are required for proper s390[x] support. With this
series, s390[x] support is basically complete. However, I have
another patch that adds complex type support in the reflection
library, but that requires the latest libffi code. I'll look into
merging that back into gcc next.

Summary of this series:
-----------------------

0001: Ports libgo to s390[x].
0002: Adapts some tests in libgo but also fixes some bugs in the
tests.
0003: Allow "//+ build" for the go tests in Gcc.
0004: s390[x] specific fixes for some tests in golang.

Ciao

Dominik ^_^ ^_^

--

Dominik Vogt
IBM Germany

[Dominik Vogt]

See commit comment and ChangeLog for details.

[Dominik Vogt]

[Dominik Vogt]

[Dominik Vogt]

[Ian Taylor]

On Tue, Nov 4, 2014 at 4:15 AM, Dominik Vogt <vo...@linux.vnet.ibm.com> wrote:
> See commit comment and ChangeLog for details.

> case "amd64", "386":
> + case "s390", "s390x":

Note that this doesn't do what you want. In Go, unlike C, cases do
not fall through by default. So doing this means that the amd64 and
386 cases do nothing at all.

I will change it to

case "amd64", "386", "s390", "s390x":

Ian

[Ian Taylor]

On Tue, Nov 4, 2014 at 4:15 AM, Dominik Vogt <vo...@linux.vnet.ibm.com> wrote:

> See commit comment and ChangeLog for details.

Committed patch 0001 with various formatting fixes, as attached.

Note that libgo/runtime/runtime.c now refers to S390_HAVE_STCKF. It's
not obvious to me that that is defined anywhere. Perhaps it is in a
later patch in this series--I haven't looked.

Ian

[Ian Taylor]

Part of this patch duplicates a patch that is already applied to the
master Go library, so I explicitly backported that patch
(https://codereview.appspot.com/111320044) instead.

Bootstrapped and ran Go testsuite on x86_64-unknown-linux-gnu.
Committed to mainline.

Ian

[Ian Taylor]

For the rest of this patch, I replied on the bug
(http://gcc.gnu.org/PR63269). I'm not persuaded that the approach
this patch takes is correct.

Ian

[Ian Taylor]

I committed this patch.

Thanks.

I changed the ChangeLog entry to this, since your ChangeLog entry
didn't seem accurate to me.

2014-11-04 Dominik Vogt <vo...@linux.vnet.ibm.com>

* go.test/go-test.exp: In +build lines, require whitespace around
expected strings, fix check for negation.

Ian

[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

[Ian Taylor]

On Tue, Nov 4, 2014 at 4:15 AM, Dominik Vogt <vo...@linux.vnet.ibm.com> wrote:
>

> This is the second set of patches to support s390[x] with Gccgo,
> containing mostly architecture dependent parts that affect the
> following directories:

> Summary of this series:
> -----------------------
>
> 0001: Ports libgo to s390[x].
> 0002: Adapts some tests in libgo but also fixes some bugs in the
> tests.
> 0003: Allow "//+ build" for the go tests in Gcc.
> 0004: s390[x] specific fixes for some tests in golang.

Thanks. I've replied to all the patches in this series.

Ian

[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.

[Dominik Vogt]

On Tue, Nov 04, 2014 at 02:39:34PM -0800, Ian Taylor wrote:
> Note that libgo/runtime/runtime.c now refers to S390_HAVE_STCKF. It's
> not obvious to me that that is defined anywhere. Perhaps it is in a
> later patch in this series--I haven't looked.

This chunk is broken but harmless (because S390_HAVE_STCKF is
never defined anyway). The code needs to check at runtime whether
the stckf instruction is available. I'll provide a patch for that
later. In the mean time there's no immediate need to back out the
flawed chunk.

[Dominik Vogt]

On Wed, Nov 05, 2014 at 11:31:28AM +0100, Dominik Vogt wrote:
> On Tue, Nov 04, 2014 at 02:39:34PM -0800, Ian Taylor wrote:
> > Note that libgo/runtime/runtime.c now refers to S390_HAVE_STCKF. It's
> > not obvious to me that that is defined anywhere. Perhaps it is in a
> > later patch in this series--I haven't looked.

The attached patch fixes this (by using stckf unconditionally).

[Ian Taylor]

On Wed, Nov 5, 2014 at 2:05 AM, Dominik Vogt <vo...@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

[Dominik Vogt]

But there is no issue in Gcc. The Go testsuite's assumption that
run time calculations must yield the same result as pre-calculated
constant calculations does not hold true in Gcc because Gcc uses
higher precision for that (libgmp).

In my eyes, the tests in cplx2.go that involve a division are
unreasonably strict, and marking cplx2.go "xfail" for s390[x] is
counterproductive because the test result is already as precise at
it will ever get.

The only way I see to "fix" this would be to *lower* Gcc's
precision of calculations for constant folding, or to disable
constant folding for Go code. (And then, this may well break
tests on other platforms).

[Dominik Vogt]

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?

[Rainer Orth]

Ian Taylor <ia...@golang.org> writes:

> On Tue, Nov 4, 2014 at 4:15 AM, Dominik Vogt <vo...@linux.vnet.ibm.com> wrote:
>> See commit comment and ChangeLog for details.
>
> Committed patch 0001 with various formatting fixes, as attached.

Unfortunately, the mksysinfo.sh part broke Solaris bootstrap: there's no
type _ucred in gen-sysinfo.go, so the grep in upcase_fields fails. Due
to the use of set -e, the whole script aborts.

I've used the following to allow bootstrap to continue

[Dominik Vogt]

On Thu, Nov 06, 2014 at 03:02:13PM +0100, Rainer Orth wrote:
> Ian Taylor <ia...@golang.org> writes:
> > Committed patch 0001 with various formatting fixes, as attached.
>
> Unfortunately, the mksysinfo.sh part broke Solaris bootstrap: there's no
> type _ucred in gen-sysinfo.go, so the grep in upcase_fields fails. Due
> to the use of set -e, the whole script aborts.

The attached patch fixes the problem.

> It seems something changed again in godump within the last few days,
> making this patch
>
> https://gcc.gnu.org/ml/gcc-patches/2014-11/msg00080.html
>
> unnecessary.

Yes. :-)

[Ian Taylor]

On Thu, Nov 6, 2014 at 7:34 AM, Dominik Vogt <vo...@linux.vnet.ibm.com> wrote:
> On Thu, Nov 06, 2014 at 03:02:13PM +0100, Rainer Orth wrote:
>> Ian Taylor <ia...@golang.org> writes:
>> > Committed patch 0001 with various formatting fixes, as attached.
>>
>> Unfortunately, the mksysinfo.sh part broke Solaris bootstrap: there's no
>> type _ucred in gen-sysinfo.go, so the grep in upcase_fields fails. Due
>> to the use of set -e, the whole script aborts.
>
> The attached patch fixes the problem.

Thanks. Committed.

Ian

[Ian Taylor]

On Wed, Nov 5, 2014 at 9:13 AM, Dominik Vogt <vo...@linux.vnet.ibm.com> wrote:
> On Wed, Nov 05, 2014 at 07:52:18AM -0800, Ian Taylor wrote:
>> On Wed, Nov 5, 2014 at 2:05 AM, Dominik Vogt <vo...@linux.vnet.ibm.com> wrote:
>> > On Tue, Nov 04, 2014 at 08:16:51PM -0800, Ian Taylor wrote:
>> >> 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.
>
> But there is no issue in Gcc. The Go testsuite's assumption that
> run time calculations must yield the same result as pre-calculated
> constant calculations does not hold true in Gcc because Gcc uses
> higher precision for that (libgmp).

For the Go language, I can't describe that as anything other than a
bug. Go has both untyped constants and typed constants. Calculations
involving typed constants follow the exact rules of the type. The Go
frontend implements this. Untyped constants are handled entirely by
the frontend and never reach the GCC middle-end. Typed constants are
computed one operation at a time and rounded back to the type after
each operation.

I admit that I have still have not looked at the actual code in GCC,
but your description makes the code sound simply wrong for Go. It is
probably acceptable for C/C++, though I doubt it is correct for Java.
And it may not be worth fixing. But I don't see any way to describe
it as correct.

That said, when I look at PR 60181, it appears to be saying something
different. It appears to be saying that the compile-time calculation
is correct, but the runtime calculation is not.

> In my eyes, the tests in cplx2.go that involve a division are
> unreasonably strict, and marking cplx2.go "xfail" for s390[x] is
> counterproductive because the test result is already as precise at
> it will ever get.

I haven't yet seen any reason to think that the tests are unreasonably
strict.

> The only way I see to "fix" this would be to *lower* Gcc's
> precision of calculations for constant folding, or to disable
> constant folding for Go code. (And then, this may well break
> tests on other platforms).

The first choice would be correct for Go. Go is not C.

Ian

[Ian Taylor]

On Wed, Nov 5, 2014 at 4:55 AM, Dominik Vogt <vo...@linux.vnet.ibm.com> wrote:
> On Wed, Nov 05, 2014 at 11:31:28AM +0100, Dominik Vogt wrote:
>> On Tue, Nov 04, 2014 at 02:39:34PM -0800, Ian Taylor wrote:
>> > Note that libgo/runtime/runtime.c now refers to S390_HAVE_STCKF. It's
>> > not obvious to me that that is defined anywhere. Perhaps it is in a
>> > later patch in this series--I haven't looked.
>
> The attached patch fixes this (by using stckf unconditionally).

Thanks. Committed.

Ian

[Ian Taylor]

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

[Dominik Vogt]

On Thu, Nov 06, 2014 at 08:57:44AM -0800, Ian Taylor wrote:
> On Wed, Nov 5, 2014 at 9:13 AM, Dominik Vogt <vo...@linux.vnet.ibm.com> wrote:
> > But there is no issue in Gcc. The Go testsuite's assumption that
> > run time calculations must yield the same result as pre-calculated
> > constant calculations does not hold true in Gcc because Gcc uses
> > higher precision for that (libgmp).
>
> For the Go language, I can't describe that as anything other than a
> bug. Go has both untyped constants and typed constants. Calculations
> involving typed constants follow the exact rules of the type. The Go
> frontend implements this. Untyped constants are handled entirely by
> the frontend and never reach the GCC middle-end. Typed constants are
> computed one operation at a time and rounded back to the type after
> each operation.

I'm not sure whether what you say is really applicable here. From
cplx2.go:

const (
R = 5
I = 6i
C1 = R + I // ADD(5,6)
C5 = C1 + R // ADD(10,6)
C6 = C1 + I // ADD(5,12)
Cd = C5 / C6 // DIV(0.721893,-0.532544)
)

The mentioned test case compares with Cd. There are three
approaches how the compiler could express that:

1) Every time Cd appears in the code with type T, generate code
to calculate

Cd T = C5 / C6 = (T(5) + T(6i) + T(5)) / (T(5) + T(6i) + T(6i))

T can be a different type every time Cd is used. (I.e.
disable pre-calculation of anonymous constants.) Only possible
if the whole definition of the constant is in the same compile
unit. This would rule out linking Go code with libraries that
define anonymous constants, or in other words, anonymous constants
cannot be exported to other compile units. (But this is done
in the math library anyway.)

2) Pre-calculate a different version of Cd for every possible
type (complex64, complex128, float32(?), int64(?), ...), then
pick the correct version every time it's used.

3) Pre-calculate Cd with the highest possible precision, then
scale it down to the type that is actually needed in the code
that uses Cd. (This is done in the frontend.)

Gcc currently does (3), and if I read your explanation correctly,
you imply that it should do (1) and must not do (2)?

--

To understand this issue better, can you point me to the specs
that require that

a) Calculations using constants in Go must have the same result as
calculations using dynamic values.
b) Multiple identical calculations of the same formula in
different places of the Go code (i.e. in different surrounding
context) must yield identical results?

Especially (b) seems to be impossible to guarantee to me, because
it depends on the code that is actually generated at the place
where the constant is used.

> I admit that I have still have not looked at the actual code in GCC,
> but your description makes the code sound simply wrong for Go. It is
> probably acceptable for C/C++, though I doubt it is correct for Java.
> And it may not be worth fixing. But I don't see any way to describe
> it as correct.
>
> That said, when I look at PR 60181, it appears to be saying something
> different. It appears to be saying that the compile-time calculation
> is correct, but the runtime calculation is not.

What do you mean? In the comments I find

>> Constant folding is correctly rounding, runtime complex
>> multiplication / division isn't (and given complaints about
>> slowness at present, I don't think users would want it to be even
>> slower, though there may well be a case for defining a standard
>> library interface for correctly rounding complex multiplication /
>> division).

and

>> you generally cannot expect constant folding and runtime execution
>> to produce the exact same result. This is FP after all ...

And regardless of the discussion above, "this is FP after all"
always wins, no?

[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 <vo...@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?

[Ian Taylor]

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

[Ian Taylor]

No, 3 is correct for an untyped constant. At the point where Cd is
used in an actual calculation, it must be rounded to the correct type.

The question is how the code should behave at run time.

> To understand this issue better, can you point me to the specs
> that require that
>
> a) Calculations using constants in Go must have the same result as
> calculations using dynamic values.
> b) Multiple identical calculations of the same formula in
> different places of the Go code (i.e. in different surrounding
> context) must yield identical results?
>
> Especially (b) seems to be impossible to guarantee to me, because
> it depends on the code that is actually generated at the place
> where the constant is used.

I'm sorry, I don't entirely understand the questions. In Go untyped
constants and typed constants are handled differently. Typed
constants should behave like runtime code. Untyped constants should
not. I don't know if it will help, but see
http://golang.org/ref/spec#Constants and
http://golang.org/ref/spec#Constant_expressions .

>> That said, when I look at PR 60181, it appears to be saying something
>> different. It appears to be saying that the compile-time calculation
>> is correct, but the runtime calculation is not.
>
> What do you mean? In the comments I find
>
>>> Constant folding is correctly rounding, runtime complex
>>> multiplication / division isn't (and given complaints about
>>> slowness at present, I don't think users would want it to be even
>>> slower, though there may well be a case for defining a standard
>>> library interface for correctly rounding complex multiplication /
>>> division).

Right, that is what I just said.

>>> you generally cannot expect constant folding and runtime execution
>>> to produce the exact same result. This is FP after all ...
>
> And regardless of the discussion above, "this is FP after all"
> always wins, no?

No, not at all. FP calculations are always precise and predictable.
The implications of when values are rounded are well understood by
people who write serious FP code, and languages should support them,
rather than behaving unpredictably.

That said, I may be overthinking how Go should behave in this context.
I will start a thread on golang-dev to express what I see as the
issue.

Ian

[Michael Hudson-Doyle]

Ian Taylor <ia...@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!

[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.

[Ian Taylor]

On Mon, Nov 10, 2014 at 6:00 AM, Dominik Vogt <vo...@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

[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 <vo...@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 <vo...@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.

[Ian Taylor]

On Thu, Nov 13, 2014 at 2:58 AM, Dominik Vogt <vo...@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

[Dominik Vogt]

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.