This works around some UB in LLVM, where an out-of-bounds conversion would produce a poison value.
The selected behavior is saturating, except that NaN is mapped to the minimum value.
This commit finally introduces unit tests for the compiler, to check
whether input Go code is converted to the expected output IR.
To make this necessary, a few refactors were needed. Hopefully these
refactors (to compile a program package by package instead of all at
once) will eventually become standard, so that packages can all be
compiled separate from each other and be cached between compiles.
Before this change, the compiler could panic with the following message:
panic: 20 not an Int
That of course doesn't make much sense. But it apparently is expected
behavior, see https://github.com/golang/go/issues/43165 for details.
This commit fixes this issue by converting the constant to an integer if
needed.
Because the parentHandle parameter wasn't always set to the right value,
the coroutine lowering pass would sometimes panic with "trying to make
exported function async" even though there was no exported function
involved. Therefore, it should unconditionally be set to avoid this.
The parent function doesn't always have the parentHandle function
parameter set because it can only be set after defining a function, not
when it is only declared.
Previously, EmitPointerPack would generate an out-of-bounds read from an
alloca. This commit fixes that by creating an alloca of the appropriate
size instead of using the size of the to-be-packed data (which might be
smaller than a pointer).
I discovered this error while working on a rewrite of the interp
package, which checks for out-of-bounds reads and writes. There I
discovered this issue when the image package was compiled.
Test binaries must be run in the source directory of the package to be
tested. This wasn't done, leading to a few "file not found" errors.
This commit implements this. Unfortunately, it does not allow more
packages to be tested as both affected packages (debug/macho and
debug/plan9obj) will still fail with this patch even though the "file
not found" errors are gone.
There were a few problems with the go/packages package. While it is more
or less designed for our purpose, it didn't work quite well as it didn't
provide access to indirectly imported packages (most importantly the
runtime package). This led to a workaround that sometimes broke
`tinygo test`.
This PR contains a number of related changes:
* It uses `go list` directly to retrieve the list of packages/files to
compile, instead of relying on the go/packages package.
* It replaces our custom TestMain replace code with the standard code
for running tests (generated by `go list`).
* It adds a dummy runtime/pprof package and modifies the testing
package, to get tests to run again with the code generated by
`go list`.
This is a big change that will determine the stack size for many
goroutines automatically. Functions that aren't recursive and don't call
function pointers can in many cases have an automatically determined
worst case stack size. This is useful, as the stack size is usually much
lower than the previous hardcoded default of 1024 bytes: somewhere
around 200-500 bytes is common.
A side effect of this change is that the default stack sizes (including
the stack size for other architectures such as AVR) can now be changed
in the config JSON file, making it tunable per application.
This is required by the coroutines pass, otherwise it will panic. It
checks for the proper parameter names to make sure the function is not
exported. In this case, the runtime.initAll function wasn't exported but
simply didn't have the correct parameter names so the check triggered
even though it shouldn't.
This is necessary to avoid a circular dependency between the device/avr
and runtime/interrupts package in the next commit.
It may be worth replacing existing calls like device/arm.Asm to
device.Asm, to have a single place where these are defined.
Previously, chansend and chanrecv allocated a heap object before blocking on a channel.
This object was used to implement a linked list of goroutines blocked on the channel.
The chansend and chanrecv now instead accept a buffer to store this object in as an argument.
The compiler now creates a stack allocation for this object and passes it in.
There were a few cases left where a named type would cause a crash in
the compiler. While going through enough code would have found them
eventually, I specifically looked for the `Type().(` pattern: a Type()
call that is then used in a type assert. Most of those were indeed bugs,
although for some I couldn't come up with a reproducer so I left them
as-is.
This commit replaces the existing ad-hoc package loader with a package
loader that uses the x/tools/go/packages package to find all
to-be-loaded packages.
This commit changes the way that packages are looked up. Instead of
working around the loader package by modifying the GOROOT variable for
specific packages, create a new GOROOT using symlinks. This GOROOT is
cached for the specified configuration (Go version, underlying GOROOT
path, TinyGo path, whether to override the syscall package).
This will also enable go module support in the future.
Windows is a bit harder to support, because it only allows the creation
of symlinks when developer mode is enabled. This is worked around by
using symlinks and if that fails, using directory junctions or hardlinks
instead. This should work in the vast majority of cases. The only case
it doesn't work, is if developer mode is disabled and TinyGo, the Go
toolchain, and the cache directory are not all on the same filesystem.
If this is a problem, it is still possible to improve the code by using
file copies instead.
As a side effect, this also makes diagnostics use a relative file path
only when the file is not in GOROOT or in TINYGOROOT.
Previously, we implemented individual bytealg functions via linknaming, and had to update them every once in a while when we hit linker errors.
Instead, this change reimplements the bytealg package in pure Go.
If something is missing, it will cause a compiler error rather than a linker error.
This is easier to test and maintain.
This makes viewing the IR easier because parameters have readable names.
This also makes it easier to write compiler tests (still a work in
progress), that work in LLVM 9 and LLVM 10, as LLVM 10 started printing
value names for unnamed parameters.
Previously, the typecode was passed via a direct reference, which results in invalid IR when the defer is not reached in all return paths.
It also results in incorrect behavior if the defer is in a loop, causing all defers to use the typecode of the last iteration.
This is a very common case. Avoiding a runtime.interfaceEqual call leads
to a very big reduction in code size in some cases (while it doesn't
affect many other examples). A number of driver smoke tests are reduced
by about 4kB just with this optimization.
I found this issue while looking into automatically calculating the
required amount of stack space for goroutines. The
runtime.interfaceEqual function is recursive, so it is best avoided.
This is used for example by the errors package, which contains:
if x, ok := err.(interface{ As(interface{}) bool }); ok && x.As(target) {
return true
}
The interface here is not a named type.
Previously, the compiler used LLVM's shift instructions directly, which have UB whenever the shifts are large or negative.
This commit adds runtime checks for negative shifts, and handles oversized shifts.
This gives the optimizer a bit more information about what the calls do.
This should result in slightly better generated code.
Code size sometimes goes up and sometimes goes down. I blame the code
size going up on the inliner which inlines more functions, because
compiling the smoke tests in the drivers repository with -opt=1 results
in a slight code size reduction in all cases.
This replaces the custom runtime.memcpy and runtime.memmove functions
with calls to LLVM builtins that should hopefully allow LLVM to better
optimize such calls. They will be lowered to regular libc memcpy/memmove
when they can't be optimized away.
When testing this change with some smoke tests, I found that many smoke
tests resulted in slightly larger binary sizes with this commit applied.
I looked into it and it appears that machine.sendUSBPacket was not
inlined before while it is with this commit applied. Additionally, when
I compared all driver smoke tests with -opt=1 I saw that many were
reduced slightly in binary size and none increased in size.
This hack was originally introduced in
https://github.com/tinygo-org/tinygo/pull/251 to fix an escape analysis
regression after https://github.com/tinygo-org/tinygo/pull/222
introduced nil checks. Since a new optimization in LLVM (see
https://reviews.llvm.org/D60047) this hack is not necessary anymore and
can be removed.
I've compared all regular tests and smoke tests before and after to
check the size. In most cases this change was an improvement although
there are a few regressions.
The unsafe.Pointer type is used for many low-level operations,
especially in the runtime. It can for example be used to copy the
contents of a slice (in the copy builtin) independent of the slice
element type.
The x/tools/go/ssa package splits slice loads/stores into two
operations. So for code like this:
x = p[3]
It has two instructions:
x_ptr = &p[3]
x = *x_ptr
This makes the IR simpler, but also means we're accidentally inserting
more nil checks than necessary: the slice index operation has
effectively already checked for nil by performing a bounds check.
Therefore, omit nil pointer checks for pointers created by
*ssa.IndexAddr.
This change is necessary to make sure a future removal of runtime.isnil
will not cause the escape analysis pass to regress. Apart from that, it
reduces code size slightly in many smoke tests (with no increases in
code size).
This gives a hint to the compiler that such parameters are either NULL
or point to a valid object that can be dereferenced. This is not
directly very useful, but is very useful when combined with
https://reviews.llvm.org/D60047 to remove the runtime.isnil hack without
regressing escape analysis.
This commit merges NewCompiler and Compile into one simplifying the
external interface. More importantly, it does away with the entire
Compiler object so the public API becomes a lot smaller.
The refactor is not complete: eventually, the compiler should just
compile a single package without trying to load it first (that should be
done by the builder package).
Nia Weiss
Ayke van Laethem
Lucas Teske
waj334
Jaden Weiss
Yannis Huber
cornelk
suzuki-koya