Teensy 4.0: Fix UART capability

[ardnew]

This PR addresses issue (#1649), where UART stopped functioning after interp package rewrite.

After the rewrite, it was observed that interp no longer supports referring to memory-mapped registers from global contexts such as package-level var and func init.

[deadprogram]

Thanks for working on this @ardnew

I tested and the serial example worked, but the echo example, even after I switched to UART1, did not.

[aykevl]

@ardnew please take a look here: #1655.

[ardnew]

@deadprogram I think the PR from @aykevl is the more robust solution here and this one should be rejected/closed. This also avoids having to rework the I2C/SPI PRs.

Neither of these resolve the issue you found with the serial example though, so I'll open a new issue for it and mark the one above as resolved (after one of these gets merged)

[deadprogram]

Still broken as of as of #1655

[aykevl]

I looked at the difference in IR, but they appear equivalent. Maybe I've missed something, though.

--- test0.txt	2021-03-05 21:29:02.014715881 +0100
+++ test1.txt	2021-03-05 21:29:09.782656409 +0100
@@ -270,7 +270,7 @@
 @"(machine.Pin).getMuxMode$pack.177" = private unnamed_addr constant { %runtime._string } { %runtime._string { i8* getelementptr inbounds ([25 x i8], [25 x i8]* @"(machine.Pin).getMuxMode$string.176", i32 0, i32 0), i32 25 } }
 @"(machine.Pin).getPad$string" = internal unnamed_addr constant [20 x i8] c"machine: invalid pin"
 @"(machine.Pin).getPad$pack" = private unnamed_addr constant { %runtime._string } { %runtime._string { i8* getelementptr inbounds ([20 x i8], [20 x i8]* @"(machine.Pin).getPad$string", i32 0, i32 0), i32 20 } }
-@machine.UART1 = internal global %machine.UART { %"device/nxp.LPUART_Type"* inttoptr (i32 1075412992 to %"device/nxp.LPUART_Type"*), %machine.RingBuffer* @"machine$alloc", %"runtime/interrupt.Interrupt" { i32 ptrtoint (%"runtime/interrupt.handle"* @"runtime/interrupt.$interrupt25" to i32) }, %machine.muxSelect { i8 1, %"runtime/volatile.Register32"* getelementptr (%"device/nxp.IOMUXC_Type", %"device/nxp.IOMUXC_Type"* inttoptr (i32 1075806208 to %"device/nxp.IOMUXC_Type"*), i32 0, i32 336) }, %machine.muxSelect { i8 1, %"runtime/volatile.Register32"* getelementptr (%"device/nxp.IOMUXC_Type", %"device/nxp.IOMUXC_Type"* inttoptr (i32 1075806208 to %"device/nxp.IOMUXC_Type"*), i32 0, i32 337) }, i8 0, i8 0, i32 0, i1 false, i1 false, %"runtime/volatile.Register32" zeroinitializer, %machine.RingBuffer* null }
+@machine.UART1 = internal global %machine.UART { %"device/nxp.LPUART_Type"* inttoptr (i32 1075412992 to %"device/nxp.LPUART_Type"*), %machine.RingBuffer* bitcast ([130 x i8]* @"machine$alloc" to %machine.RingBuffer*), %"runtime/interrupt.Interrupt" { i32 ptrtoint (%"runtime/interrupt.handle"* @"runtime/interrupt.$interrupt25" to i32) }, %machine.muxSelect { i8 1, %"runtime/volatile.Register32"* inttoptr (i32 1075807568 to %"runtime/volatile.Register32"*) }, %machine.muxSelect { i8 1, %"runtime/volatile.Register32"* inttoptr (i32 1075807572 to %"runtime/volatile.Register32"*) }, i8 0, i8 0, i32 0, i1 false, i1 false, %"runtime/volatile.Register32" zeroinitializer, %machine.RingBuffer* null }
 @"runtime/interrupt.$interrupt25" = private unnamed_addr constant %"runtime/interrupt.handle" { { i8*, void (i32, i8*, i8*)* } { i8* bitcast (%machine.UART* @machine.UART1 to i8*), void (i32, i8*, i8*)* @"(*machine.UART).handleInterrupt$bound" }, %"runtime/interrupt.Interrupt" { i32 25 } }
 @"runtime.GC$string" = internal unnamed_addr constant [27 x i8] c"running collection cycle..."
 @runtime.runqueue = internal global %"internal/task.Queue" zeroinitializer
@@ -289,7 +289,6 @@
 @runtime.poolStart = internal global i32 0
 @"runtime.blockFromAddr$string" = internal unnamed_addr constant [44 x i8] c"gc: trying to get block from invalid address"
 @runtime.heapEnd = internal global i32 ptrtoint ([0 x i8]* @_heap_end to i32)
-@runtime.inf = internal global double 0.000000e+00
 @"runtime.dumpHeap$string" = internal unnamed_addr constant [5 x i8] c"heap:"
 @"runtime.dumpHeap$string.199" = internal unnamed_addr constant [1 x i8] c"*"
 @"runtime.dumpHeap$string.200" = internal unnamed_addr constant [1 x i8] c"-"
@@ -417,8 +416,7 @@
 @"(*internal/task.Queue).Push$string" = internal unnamed_addr constant [62 x i8] c"runtime: pushing a task to a queue with a non-nil Next pointer"
 @"(*internal/task.Queue).Push$pack" = private unnamed_addr constant { %runtime._string } { %runtime._string { i8* getelementptr inbounds ([62 x i8], [62 x i8]* @"(*internal/task.Queue).Push$string", i32 0, i32 0), i32 62 } }
 @tinygo_startTask = external global [0 x i8]
-@"machine$alloc" = internal global %machine.RingBuffer zeroinitializer
-@"runtime$alloc" = internal global i64 9218868437227405312
+@"machine$alloc" = internal global [130 x i8] zeroinitializer, align 4
 
 define internal i32 @"(reflect.Type).Elem"(i32 %t, i8* %context, i8* %parentHandle) unnamed_addr {
 entry:
@@ -9527,9 +9525,6 @@
   %156 = call i32 @"runtime/interrupt.Register"(i32 155, i8* getelementptr inbounds ([22 x i8], [22 x i8]* @"device/nxp.init$string.165", i32 0, i32 0), i32 22, i8* undef, i8* undef)
   %157 = call i32 @"runtime/interrupt.Register"(i32 156, i8* getelementptr inbounds ([18 x i8], [18 x i8]* @"device/nxp.init$string.166", i32 0, i32 0), i32 18, i8* undef, i8* undef)
   %158 = call i32 @"runtime/interrupt.Register"(i32 157, i8* getelementptr inbounds ([22 x i8], [22 x i8]* @"device/nxp.init$string.167", i32 0, i32 0), i32 22, i8* undef, i8* undef)
-  %159 = load double, double* bitcast (i64* @"runtime$alloc" to double*)
-  store double %159, double* @runtime.inf
-  %dummy = alloca i8
   ret void
 }

This is the difference in output (using tinygo build -o test.hex -target=teensy40 -printir -no-debug examples/serial) with the following patch applied:

diff --git a/builder/build.go b/builder/build.go
index 3e7ce62a..5b57b3b2 100644
--- a/builder/build.go
+++ b/builder/build.go
@@ -54,10 +54,6 @@ func Build(pkgName, outpath string, config *compileopts.Config, action func(Buil
        }
        mod := buildOutput.Mod
 
-       if config.Options.PrintIR {
-               fmt.Println("; Generated LLVM IR:")
-               fmt.Println(mod.String())
-       }
        if err := llvm.VerifyModule(mod, llvm.PrintMessageAction); err != nil {
                return errors.New("verification error after IR construction")
        }
@@ -66,6 +62,14 @@ func Build(pkgName, outpath string, config *compileopts.Config, action func(Buil
        if err != nil {
                return err
        }
+       goPasses := llvm.NewPassManager()
+       defer goPasses.Dispose()
+       goPasses.AddGlobalDCEPass()
+       goPasses.Run(mod)
+       if config.Options.PrintIR {
+               fmt.Println("; Generated LLVM IR:")
+               fmt.Println(mod.String())
+       }
        if err := llvm.VerifyModule(mod, llvm.PrintMessageAction); err != nil {
                return errors.New("verification error after interpreting runtime.initAll")
        }

The "before" IR of the IR diff above is at e9d549d (right before the interp rewrite) while the "after" IR is at 5917b8b with 57a2dd0 (interp-mmio-gep branch) cherry picked on top (so 3 commits difference).

Is examples/serial a good test case?

[aykevl]

Digging deeper, I'm pretty sure those two IRs are in fact equivalent (apart from the load double / store double stuff which is unrelated). So for those who have a Teensy 4.0, can you test the following:

• Whether it works on e9d549d (as a sanity check)
• Whether it works on the tmp-teensy40 branch at f50758c.

[ardnew]

@aykevl Indeed even with sanity check (e9d549d), RX seems to not function correctly in the echo example. So this aspect seems to be a pre-existing issue. However, TX was working at this commit.

If you merge my last changes (c27235b) with your tmp-teensy40 branch (you don't need my changes from a371265 in this case), then TX functionality is restored.

I'll continue researching the issue with RX.

[deadprogram]

OK took a little while longer than expected, but finally got setup. Can confirm that merging this branch on top of dev along with #1655 does indeed restore the TX on the Teensy4.0

[aykevl]

@ardnew I think you wanted to revert the first commit of this PR? It should no longer be needed with #1655, right?

[ardnew]

@ardnew I think you wanted to revert the first commit of this PR? It should no longer be needed with #1655, right?

So I actually kinda liked the changes in the first commit. They simplify the machine package by removing a type (and Teensy 4.0 desperately needs to trim some fat), and it eliminates the dependence on your MM-I/O enhancement, which realistically isn't time-tested (yet).

However, for the purpose of addressing the issue (#1649), it is definitely an unnecessary change when combined with your updates. Furthermore, reverting that commit will also avoid any changes to the other Teensy 4.0 peripherals - SPI (#1455) and I2C (#1471), in particular.

So I'll move the changes from the first commit to a "Teensy 4.0 cleanup/simplify" branch for a future PR, and revert it from this one.

[deadprogram]

I think you need to rebase against the latest dev branch, if you please.

[ardnew]

@deadprogram Rebased against dev, and also added an alias UART0 for UART1. This way the src/examples/echo demo compiles and runs without modification

[deadprogram]

Thanks for the fix and the port alias. Now merging.