LLVM 15

[aykevl]

Work in progress branch. This LLVM version introduces a ton of changes that we somehow have to deal with. I'm splitting it up in many separate commits so that we can more easily bisect and track regressions.

[aykevl]

Ready for review. There seems to be an issue with MacOS which I'll need to fix but other than that this is ready from my point of view. EDIT: actually it appears that LLVM 15 still has issues in practice.

This will switch to LLVM 15 by default when using go install, but doesn't yet update the compiler tests (go test ./compiler). I've done this because most tests change and I want to keep testing them in LLVM 14 in CI (the Espressif fork hasn't been published yet). If you want to run compiler tests, you're probably better off with go test -tags=llvm14 ./compiler for now.

I verified that even with these rather extensive changes, LLVM 14 builds produce the same binaries as before (tested using the drivers repo smoke tests - the only exceptions were AVR and Xtensa that currently unintentionally embed the TinyGo version in the binaries).

@QuLogic I think this is relevant for you.

[QuLogic]

Thanks for the heads-up. I tried backporting to 0.25.0, but that crashed in make test. I guess there's some other updates that I'd be missing there. I will try again with 0.26.0 once I've done updating to it, but if that doesn't work, I'll see if I can use llvm15 directly.

[aykevl]

@QuLogic unfortunately this PR isn't ready yet, I hit a difficult bug that I need to fix before this PR becomes usable (hence why I've marked it as a draft). Most code works, I think it's only wasm that fails. But I wouldn't suggest using this patch in Fedora because TinyGo is very often used for wasm.

[QuLogic]

Seems like there's still a SIGSEGV in EraseFromParentAsInstruction even with it moved around from 05b3c51.

[aykevl]

Yeah, I also found it with some more testing. I hope I've now fixed all memory corruption bugs - at least, it doesn't complain anymore in my last tests with AddressSanitizer enabled.

[aykevl]

Nevermind, tinygo run -target=wasi ./testdata/testing.go still fails.

[aykevl]

Next try. That last bug was particularly gnarly but should be fixed with #3230 (also included in this PR).

[QuLogic]

Good work; it works for me backported to 0.26.0 so far.

[deadprogram]

Interestingly, it appears that the Itsybitsy-M4 I2C tests are failing with this PR. I retried several times which all failed. Other recent PRs are all passing, and dev branch also passing. I verified that the dev branch passed tests after this branch failed, and then retried this PR again after it passed on dev. So it does appear to be something not working correctly.

[aykevl]

I can reproduce this issue locally on the ItsyBitsy M4 (after I pulled that little board out of a project...). So I'm afraid that's yet another bug to fix...

[aykevl]

For some reason, waiting for the t character causes the I2C test to fail, suggesting that the bug could also be in the USB-CDC parts.

[aykevl]

Update: I haven't figured out the root cause here, but I did find that when I connect the MPU6050 directly to a 3.3V output the 2nd test that was failing will pass. Perhaps the underlying issue is a power issue? After all, these pins can supply only 2mA with the default configuration (see table 54-15 in the datasheet). This could be fixed with a MOSFET, for example.

No idea why the LLVM 15 update would cause it to fail though.

[deadprogram]

There has to be some explanation related to software, but it is certainly not obvious what it would be. Perhaps you can bring out your debugger to see if you can notice anything interesting.

[aykevl]

I modified the TinyHCI test a little bit to show the error message coming from accel.Configure() (see tinygo-org/drivers#473):

        powerpin.High()
        time.Sleep(100 * time.Millisecond)
 
-       accel.Configure()
+       err := accel.Configure()
+       if err != nil {
+               printtestresult(err.Error())
+               return
+       }
        time.Sleep(100 * time.Millisecond)
 
        if !accel.Connected() {

This results in the following test result in the dev branch (with LLVM 14!):

=== TINYGO INTEGRATION TESTS ===
Press 't' key to begin running tests...
- digitalReadVoltage = ***pass***
- digitalReadGround = ***fail***
- digitalWriteOn = ***pass***
- digitalWriteOff = ***fail***
- analogReadVoltage = ***fail***
        expected: 'val >= 65535-4096'
        actual: 31123
- analogReadGround = ***fail***
        expected: 'val <= 65535/2+4096 && val >= 65535/2-4096'
        actual: 29555
- analogReadHalfVoltage = ***pass***
- i2cConnectionNoPower = ***pass***
- i2cConnectionPower = ***I2C bus error***
- spiTx = ***pass***
- spiRx = ***fail***

### Tests complete.

Most test fail and that is to be expected because I only have the mpu6050 connected, but the important part here is I2C bus error. This tells me that there was something wrong even before the LLVM 15 PR.
Still figuring out what's going on here...

[aykevl]

With some more testing, I found:

• This bus error is flaky. Sometimes it appears, sometimes it doesn't. This suggests some timing problem.
• When I try the same code (with extra error checking) in LLVM 15, the bus error occurs in all cases I tested.
• When I apply a one-line fix to TinyHCI, these errors disappear in both LLVM 14 and LLVM 15.

This one line fix is the following:

        // turn on power and should be connected now
        powerpin.High()
        time.Sleep(100 * time.Millisecond)
+       machine.I2C0.Configure(machine.I2CConfig{})
 

So my best theory now is that the samd51I2C peripheral gets confused when the mpu6050 powers up but in practice it just barely worked (and we didn't see a rather important error message). Now with LLVM 15, some timing has changed making it more likely that this bug presents itself in practice. The I2C peripheral can be un-confused (reset) by reconfiguring it.

The full patch is the following:

@@ -268,8 +269,13 @@ func i2cConnection() {
        // turn on power and should be connected now
        powerpin.High()
        time.Sleep(100 * time.Millisecond)
+       machine.I2C0.Configure(machine.I2CConfig{})
 
-       accel.Configure()
+       err := accel.Configure()
+       if err != nil {
+               printtestresult(err.Error())
+               return
+       }
        time.Sleep(100 * time.Millisecond)
 
        if !accel.Connected() {

[deadprogram]

Update PR for TinyHCIL tinygo-org/tinyhci#7

Running that branch, the ItsyBitsy-M4 tests now pass.

[aykevl]

Awesome! This has been the most difficult LLVM update, by far. Hopefully nothing else breaks with this update.

[aykevl]

I had to make one last change to get make test-corpus to work, but now that also works! The necessary commit is "interp: add support for constant icmp instructions".

[aykevl]

I found that the equivalent of make test-corpus-wasi doesn't work (timeout), but it also doesn't work in the current dev branch so that's not a regression.

[aykevl]

Rebased to fix merge conflict after #3230 was merged.

[aykevl]

Anyway, this is about as ready as it gets. I can perhaps move some commits into separate PRs but other than that it's simply just a massive PR, not much I can do about it.

[deadprogram]

That took me a long time to read thru those changes. If they had not been broken up into separate commits I could not have probably even gotten thru it all!

As you said, it is a giant bunch of commits, but thank you for keeping them so organized.

As far as I am concerned, let's merge and then debug if/when needed.