tinygo-org / tinygo

Go compiler for small places. Microcontrollers, WebAssembly (WASM/WASI), and command-line tools. Based on LLVM.
https://tinygo.org
Other
15.22k stars 897 forks source link

compiler: regression failure for build on Adafruit Clue demo with ST7789 display #1765

Closed deadprogram closed 3 years ago

deadprogram commented 3 years ago

A regression bug was introduced with e2f532709f97b37575d4216c9605323221b63b93 as determined using git bisect:

$ tinygo flash -size short -target=clue .
Call parameter type does not match function signature!
{ i8, i8, i8, i8 } %5
 %"image/color.CMYK" = type { i8, i8, i8, i8 }  %9 = call fastcc %runtime._interface @"(*tinygo.org/x/drivers/st7789.Device).FillRectangle"(%"tinygo.org/x/drivers/st7789.Device"* %8, i16 %1, i16 %2, i16 %3, i16 %4, { i8, i8, i8, i8 } %5, i8* %6, i8* %7), !dbg !9963
Call parameter type does not match function signature!
{ i8, i8, i8, i8 } %3
 %"image/color.CMYK" = type { i8, i8, i8, i8 }  call fastcc void @"(*tinygo.org/x/drivers/st7789.Device).SetPixel"(%"tinygo.org/x/drivers/st7789.Device"* %6, i16 %1, i16 %2, { i8, i8, i8, i8 } %3, i8* %4, i8* %5), !dbg !9963
error: optimizations caused a verification failure

The code was that I used in my FOSDEM 2021 talk on the Adafruit Clue: https://github.com/deadprogram/fosdem-2021/blob/main/demo/scanner-ble/main.go

kenbell commented 3 years ago

I've seen some 'odd' stuff - but went away with tinygo clean. Possibly a cache invalidation issue?

aykevl commented 3 years ago

I can reproduce this issue, but with a different error:

$ tinygo flash -size short -target=clue .
Call parameter type does not match function signature!
{ i8, i8, i8, i8 } %3
 %machine.ACMFunctionalDescriptor = type { i8, i8, i8, i8 }  call fastcc void @"(*tinygo.org/x/drivers/st7789.Device).SetPixel"(%"tinygo.org/x/drivers/st7789.Device"* %6, i16 %1, i16 %2, { i8, i8, i8, i8 } %3, i8* %4, i8* %5), !dbg !22170
error: optimizations caused a verification failure
aykevl commented 3 years ago

Apparently there is a bug in the interface lowering pass. I'll need to dig a bit deeper to find out.

I have a LLVM build with assertions enabled, which is convenient to find such bugs. In this case it's triggered in the interface lowering pass (but it's possible the bug is actually somewhere else).

aykevl commented 3 years ago

~@deadprogram can you maybe try this patch? It works for me to get it to compile again. I'll write a more proper patch soon, this is just to verify I am fixing the right bug.~

diff --git a/transform/interface-lowering.go b/transform/interface-lowering.go
index 3da4f0f4..5bc386c6 100644
--- a/transform/interface-lowering.go
+++ b/transform/interface-lowering.go
@@ -703,6 +703,15 @@ func (p *lowerInterfacesPass) createInterfaceMethodFunc(itf *interfaceInfo, sign
                        // function.
                        receiver = p.builder.CreateBitCast(receiver, function.FirstParam().Type(), "")
                }
+               paramTypes := []llvm.Type{receiver.Type()}
+               for _, param := range params {
+                       paramTypes = append(paramTypes, param.Type())
+               }
+               calledFunctionType := function.Type().ElementType()
+               sig := llvm.PointerType(llvm.FunctionType(calledFunctionType.ReturnType(), paramTypes, false), 0) // TODO AVR support
+               if sig != function.Type() {
+                       function = p.builder.CreateBitCast(function, sig, "")
+               }
                retval := p.builder.CreateCall(function, append([]llvm.Value{receiver}, params...), "")
                if retval.Type().TypeKind() == llvm.VoidTypeKind {
                        p.builder.CreateRetVoid()
aykevl commented 3 years ago

I have a fix here: https://github.com/tinygo-org/tinygo/pull/1768

aykevl commented 3 years ago

This was merged in #1768.

deadprogram commented 3 years ago

Reopening until next release.

aykevl commented 3 years ago

https://github.com/tinygo-org/tinygo/commit/e2f532709f97b37575d4216c9605323221b63b93 is not in the release branch, so I believe this bug can be closed.

deadprogram commented 3 years ago

@aykevl you are correct. Now closing.