Tinygo: machine/arduino: LLVM ERROR: Not supported instr when using i2c drivers.

[deadprogram]

When attempting to build any of the i2c-based examples in the TinyGo drivers repo using the Arduino target, such as https://github.com/tinygo-org/drivers/blob/master/examples/bmp180/main.go I receive the following error:

$ tinygo build -target=arduino -o=flash.hex ./examples/bmp180/main.go                                                  
LLVM ERROR: Not supported instr: <MCInst 176 <MCOperand Reg:49> <MCOperand Reg:49>>

[aykevl]

This is a bug / missing feature in the AVR backend. To be precise, it is missing support for sdiv i64. Here is how I found the issue:

Build an IR file:

tinygo build -o test.ll -opt=1 -target=arduino -no-debug github.com/tinygo-org/drivers/examples/bmp180

opt=1 avoids inlining and -no-debug avoids debugging symbol which only get in the way for what we're doing here.

Open this file. I already had a suspicion it's in a division instruction so I looked for it. It is possible to remove the body of a function to avoid emitting certain instructions. For example, this is how I changed time.Sleep:

define internal fastcc void @time.Sleep(i64) unnamed_addr #1 section ".text.time.Sleep" {
entry:
  %1 = sdiv i64 %0, 16777216
  %2 = trunc i64 %1 to i32
  tail call fastcc void @runtime.sleepTicks(i32 %2)
  ret void
}

declare void @time.Sleep(i64) unnamed_addr #1 section ".text.time.Sleep"

This wasn't the first function I modified in this way, but it was the one after which the file started to compile, so this one must contain the offending instruction.

You can manually compile such an IR file with this command:

llc-7 -filetype=obj test.ll

To make a reproducable case, I extracted the time.Sleep function. This is the whole IR file for the repro:

source_filename = "github.com/tinygo-org/drivers/examples/bmp180"
target datalayout = "e-P1-p:16:8-i8:8-i16:8-i32:8-i64:8-f32:8-f64:8-n8-a:8"
target triple = "avr-atmel-none"

; Function Attrs: nounwind
define void @time.Sleep(i64) {
entry:
  %1 = sdiv i64 %0, 16777216
  %2 = trunc i64 %1 to i32
  tail call fastcc void @runtime.sleepTicks(i32 %2)
  ret void
}

declare void @runtime.sleepTicks(i32)

And simplified it further to:

source_filename = "github.com/tinygo-org/drivers/examples/bmp180"
target datalayout = "e-P1-p:16:8-i8:8-i16:8-i32:8-i64:8-f32:8-f64:8-n8-a:8"
target triple = "avr-atmel-none"

; Function Attrs: nounwind
define void @time.Sleep(i64) {
entry:
  %1 = sdiv i64 %0, 16777216
  tail call fastcc void @runtime.sleepTicks(i64 %1)
  ret void
}

declare void @runtime.sleepTicks(i64)

And further:

source_filename = "github.com/tinygo-org/drivers/examples/bmp180"
target datalayout = "e-P1-p:16:8-i8:8-i16:8-i32:8-i64:8-f32:8-f64:8-n8-a:8"
target triple = "avr-atmel-none"

; Function Attrs: nounwind
define i64 @sdiv(i64) {
entry:
  %1 = sdiv i64 %0, 16777216
  ret i64 %1
}

There is only one real instruction left here so this instruction must be the offending instruction.

Testing it again with LLVM 8 shows that the bug has been fixed there. In other words, when we upgrade to LLVM 8 (my estimate: ~1 week from now), this should be fixed.

[aykevl]

I've had the exact same bug before, and I worked around it by sleeping for the same duration every time.

[deadprogram]

Waiting for the next update to LLVM 8 seems worthwhile, then.

[deadprogram]

Just tested on the latest dev build, and this problem still seems to exist on LLVM8.

$ tinygo build -target arduino -o flash.hex ./examples/bmp180/main.go
LLVM ERROR: Not supported instr: <MCInst 192 <MCOperand Reg:49> <MCOperand Reg:49>>

Interestingly, this only seems to affect the BMP180 driver, because other drivers can build:

$ tinygo build -target arduino -o flash.hex ./examples/mma8653/main.go

[aykevl]

Weird, I don't know why it worked for me then. It doesn't work for me anymore now.

I think I was actually testing with a LLVM just after the LLVM 8 branch point (2019-01-19), for example with this change included: https://github.com/llvm-mirror/llvm/commit/a3db7ac3457f887631d1e8f4d244cdd20d93cbe1. It looks like it might have fixed the error.

[aykevl]

Note that when all calls to time.Sleep have the same parameter (which is obviously the case when there is just one call), the parameter will get const-propagated and the division will be avoided. That's why it works in some examples but doesn't work in others. You can test this by inserting a time.Sleep with a different duration anywhere in a working example.

[aykevl]

This should now be fixed in LLVM 8.0.1 (included in the latest release). Please reopen if it is not.

[parasquid]

Hello, sorry but I may have to reopen this. I'm testing this on the blinky2.go example on an arduino uno compatible device (a Cytron Uno) with two LEDs hooked up on D12 and D13 (I've changed the code to reflect the new pins).

Leaving it as it is with 420 as the sleep time gives:

[~/parasquid/tinygo] (dev) tristan$ tinygo flash -target arduino -port /dev/ttyUSB0 ./src/examples/blinky2/blinky2.go
LLVM ERROR: Not supported instr: <MCInst 192 <MCOperand Reg:49> <MCOperand Reg:49>>

However, making all the sleep times the same results in a successful compile.

[parasquid]

I'm currently at llvm 8.0.1 with clang-8

[~/parasquid/tinygo] (dev) tristan$ llvm-config --version
8.0.1