Tracking issue: Building and running "standard" go tests compiled with llgo

[quarnster]

Just a place to discuss this so that I can easier find it in the future.

[quarnster]

I'd like to work on this just so that we can verify that all the basics work as expected and if they don't we can start fixing them. Do you have any thoughts about how you'd like to accomplish this? Without doing much research I'm thinking either a custom llgo-test command or making the "normal" go command aware of llgo. I think the former would be better/quicker in the short term until llgo reaches maturity as it then doesn't require modifying source code outside of this repository.

[quarnster]

Or actually, adding a new compiler to the regular go command doesn't look that invasive from a quick peek in the sources.

[axw]

So I may have misunderstood your previous question. When you say the "standard go tests", are you referring to the top-level "test" directory that lives in the Go source tree (this is what I thought you meant)? Or are you talking about running pkg/testing-based tests (this is what I now think you mean)?

I broached the topic of additional compiler support in the go command a while ago, on golang-dev. See:
https://groups.google.com/forum/#!searchin/golang-dev/llgo/golang-dev/doky2Xz0sNw/ZbgAu88XD-YJ

I don't think adding another "toolchain" in the go command will fly with the Go team, not with llgo at its current level of maturity at any rate. You'll also nee to update go/build to support resolving package paths (or somehow mangle llgo to conform to what's existing in go/build).

It's not ideal, but I think it's probably quicker and easier just to write something customised. llgo-build exists for this reason.

[quarnster]

I wasn't aware of the top level test directory, but that looks like an even better start at this stage. Adding llgo as a toolchain in the go command turned out to be trickier than first assumed so I think the custom tool would be quicker also.

For the top level test directory, I think we just need to create a modified test/run.go for llgo and the question then would be where to put it. IMO it shouldn't be in cmd as "regular users" not developing llgo are unlikely to want to run it, but it might be a good idea to turn it into a xxx_test.go file which could then be tested by the drone.io builder. What do you think?

I'll start working adapting it for llgo.

[axw]

For the top level test directory, I think we just need to create a modified test/run.go for llgo and the question then would be where to put it. IMO it shouldn't be in cmd as "regular users" not developing llgo are unlikely to want to run it, but it might be a good idea to turn it into a xxx_test.go file which could then be tested by the drone.io builder. What do you think?

I think that sounds great; perhaps a test in llgo/llgo. Agreed on it not going into cmd.

I'll start working adapting it for llgo.

Thanks!

[quarnster]

Some of the tests use packages not currently built by llgo-dist, one of them being bytes. I notice that there's a bytes_libc.go in there using some #llgo name: directives. Are those name directives implemented? And if so where?

I'm getting:

Undefined symbols for architecture x86_64:
  "_bytes.Equal", referenced from:
      _bytes.Count in llgo-ovYbqN.o
      _bytes.Index in llgo-ovYbqN.o
      _bytes.LastIndex in llgo-ovYbqN.o
      _bytes.genSplit in llgo-ovYbqN.o
      _bytes.HasPrefix in llgo-ovYbqN.o
      _bytes.HasSuffix in llgo-ovYbqN.o
     (maybe you meant: _bytes.EqualFold, _bytes.Equal2 )
  "_bytes.IndexByte", referenced from:
      _bufio.*Reader.ReadSlice in llgo-ovYbqN.o
      _bufio.ScanLines in llgo-ovYbqN.o
      _bytes.*Buffer.readSlice in llgo-ovYbqN.o
      _bytes.Count in llgo-ovYbqN.o
      _bytes.Index in llgo-ovYbqN.o
     (maybe you meant: _bytes.IndexByte1)

so if the directive is indeed implemented it needs to add a _ prefix on darwin.

[axw]

"#llgo" directives (attributes) are defined in attribute.go

I'm not so sure this is the same "_" issue as in the C files, because the attribute is setting an IR name, not a symbol name. Having said that, I don't know what the issue is there.

[quarnster]

Using llvm-dis on the generated bytes.bc, seems there's a number appended at the end define i64 @bytes.IndexByte1(%15, i8), define i1 @bytes.Equal2(%15, %15), though the string passed to fn.SetName in attribute.go:128 is the correct one.

[quarnster]

This appears to be expected behavior:

setName() - Change the name of the value, choosing a new unique name if the provided name is taken.

Moot point after #42 is implemented I guess.

[axw]

Moot point after #42 is implemented I guess.

I don't think so; this is sane behaviour, and would be carried over to any pure-Go IR writer.

The real problem here is that there are two declarations of IndexByte and Equal, and there shouldn't be. This is because pkg/bytes/bytes_decl.go is declaring the functions, and then they're redeclared in bytes_libc.go. Somehow, what needs to happen, is for the declaration from bytes_decl.go to be dropped, and the bytes_libc.go to take its place.

[quarnster]

There's takeName in the C++ api for that, but I don't think it's exported in the C api. Again a moot point when #42 is implemented as takeName could just be written in the new api ;)

[axw]

Ah. Good point :)

[quarnster]

Regarding a "llgo-test", I think it's better to just have a "-test" flag in llgo-build as there's much overlap for these two activities. I'm looking into this now.

[axw]

Regarding a "llgo-test", I think it's better to just have a "-test" flag in llgo-build as there's much overlap for these two activities. I'm looking into this now.

I would be happy with that.

BTW, thanks for filing all of the issues. I haven't had much time to look at them in detail yet. If you plan to look into some, please add a comment or assign yourself so I know not to duplicate effort.

[quarnster]

If you plan to look into some, please add a comment or assign yourself so I know not to duplicate effort.

Will do. This issue depends on getting #43 working first as, which I just found out, otherwise _test.go sources aren't available in the typecheck, and if the tests use the special package name <package>_test the typecheck complains that there is no such package when it hits its import.

[quarnster]

For the bytes.IndexByte name conflict issue, doing something like this:

        curr := fn.GlobalParent().NamedFunction(name)
        if !curr.IsNil() {
            curr.SetName(name + "_llgo_replaced")
            curr.ReplaceAllUsesWith(fn)
        }
        fn.SetName(name)

in nameAttribute's Apply works. Would you rather keep the functionality of name`` as is and have this new logic in a newtake_name``` attribute?

[axw]

Nice. I think that's fine in nameAttribute, however I'd like to see a check that the function it's replacing has no body. Does that sound sensible to you?

[quarnster]

Yup

[quarnster]

So with #90 I'm getting this far:

$ llgo-build -test strings && ./strings -test.v
2013/10/04 13:30:01 building strings
clang-3.3: warning: argument unused during compilation: '-pthread'
=== RUN TestReader

Panic:
    convertI2V(interface{}, *testing.T) failed

Any suggestion on what might be amiss?

Other packages segfault:

 $ llgo-build -test path && gdb --args ./path -test.v
2013/10/04 13:33:13 building path
clang-3.3: warning: argument unused during compilation: '-pthread'
GNU gdb (GDB) 7.5.1
Copyright (C) 2012 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "x86_64-apple-darwin12.3.0".
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>...
Reading symbols from /Users/quarnster/code/go/src/github.com/axw/llgo/cmd/llgo-dist/path...done.
(gdb) r
Starting program: /Users/quarnster/code/go/src/github.com/axw/llgo/cmd/llgo-dist/path -test.v
=== RUN TestMatch
[New Thread 0x1903 of process 26843]

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x1903 of process 26843]
0x0000000100144176 in runtime.compareI2I (atyp_=0, btyp_=0, aval=0, bval=0) at interfaces.go:13
13          algs := unsafe.Pointer(atyp.alg)
(gdb) bt
#0  0x0000000100144176 in runtime.compareI2I (atyp_=0, btyp_=0, aval=0, bval=0) at interfaces.go:13
#1  0x000000010000463c in path.TestMatch (t=0x10052e5f0) at match_test.go:75
#2  0x000000010000f59b in testing.tRunner (test=0x10052bdc0) at testing.go:389
#3  0x00000001000116c4 in __unnamed_434 ()
#4  0x00000001001451f2 in runtime.callniladic (f=4295038640) at panic.go:28
#5  0x0000000100148f7d in runtime.guardedcall0 ()
#6  0x0000000100148d67 in call_gofunction ()
#7  0x00007fff87fff772 in _pthread_start () from /usr/lib/system/libsystem_c.dylib
#8  0x00007fff87fec1a1 in thread_start () from /usr/lib/system/libsystem_c.dylib
#9  0x0000000000000000 in ?? ()

$ llgo-build -test fmt && gdb --args ./fmt -test.v
2013/10/04 13:34:05 building fmt
clang-3.3: warning: argument unused during compilation: '-pthread'
GNU gdb (GDB) 7.5.1
Copyright (C) 2012 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "x86_64-apple-darwin12.3.0".
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>...
Reading symbols from /Users/quarnster/code/go/src/github.com/axw/llgo/cmd/llgo-dist/fmt...done.
(gdb) r
Starting program: /Users/quarnster/code/go/src/github.com/axw/llgo/cmd/llgo-dist/fmt -test.v

Program received signal SIGSEGV, Segmentation fault.
0x0000000100186916 in sync/atomic.CompareAndSwapUint32 ()
(gdb) bt
#0  0x0000000100186916 in sync/atomic.CompareAndSwapUint32 ()
#1  0x000000010017c5bf in sync.*Mutex.Lock (m=0x0) at mutex.go:43
#2  0x0000000100004635 in fmt.*cache.get (c=0x0) at print.go:145
#3  0x000000010000de83 in fmt.newPrinter () at print.go:165
#4  0x000000010000e231 in fmt.Sprintf (format=0x000000000000000200000001001de4e8, a=0x00000000000000010000000000000001000000010062b9f0) at print.go:234
#5  0x0000000100052b52 in flag.*stringValue.String (s=0x10062b9d0) at flag.go:192
#6  0x0000000100053f23 in flag.*FlagSet.Var (f=0x10062b7c0, value=0x0000000100052ad00000000100052a50000000010062b9d000000001001df600, name=0x000000000000000a00000001001d0d18,
    usage=0x000000000000002e00000001001d0d30) at flag.go:663
#7  0x0000000100054dcc in flag.*FlagSet.StringVar (f=0x10062b7c0, p=0x10062b9d0, name=0x000000000000000a00000001001d0d18, value=0x00000000000000000000000000000000,
    usage=0x000000000000002e00000001001d0d30) at flag.go:580
#8  0x0000000100054efa in flag.*FlagSet.String (f=0x10062b7c0, name=0x000000000000000a00000001001d0d18, value=0x00000000000000000000000000000000, usage=0x000000000000002e00000001001d0d30) at flag.go:593
#9  0x0000000100054f5f in flag.String (name=0x000000000000000a00000001001d0d18, value=0x00000000000000000000000000000000, usage=0x000000000000002e00000001001d0d30) at flag.go:600
#10 0x0000000100040aba in __llgo.ctor.testing.0 ()
#11 0x0000000100186846 in runtime.ccall ()
#12 0x0000000100181dbc in runtime.main (argc=2, argv=0x7fff5fbfef98, envp=0x7fff5fbfefb0, mainmain=0x100600160 "`\204\001") at main.go:28
#13 0x00000001000184f3 in main ()
(gdb)

Many other packages will not build due to unresolved externals, presumably due to non-translated assembly code.

[quarnster]

I consider this one done, lets open up separate issues for individual test failures or other improvements that should be made to either llgo-build -test or llgo/standard_test.go.