AVR: any way to hookup an interrupt?

[deadprogram]

I'd like to be able to use an interrupt to handle buffering of incoming UART data. Basically the same as this:

ISR (USART_RX_vect) 
{
   // read register to get next RX byte and add to ring buffer
}

Is there a way to do this?

[aykevl]

Not yet. There is some support for ARM (using //go:export), but not yet for AVR.

What needs to be done is adding a special attribute to the function and somehow hooking it up into the ISR vector. While that is done, it's probably best to also auto-generate the interrupt vector for AVR from the .atdf files (it's now hardcoded at the interrupt vector for the ATmega328). It will probably look like this commit but without the .bss/.data setup (which is already done) and with an extra //go:interrupt or //go:signal pragma.

[deadprogram]

Looking at that commit, I would for sure need some help with this task. These are pragmas I am not familiar with, among other needed learning to make progress on this enhancement.

[aykevl]

Yeah, it's a bit involved and requires knowledge of how all these parts work. If you want to work on it, here are some pointers:

• Assembly files should be generated in gen-device-avr.py. These assembly files should have the interrupt vector as rjmp instructions to a weak ISR symbol that has an alias to a default handler. This default handler just sleeps or loops endlessly (I generally prefer sleeping as this way an uncaught interrupt won't drain a battery-powered device). The wdt handler should probably be a real symbol and be kept in avr.S.

• Pragmas can be set in ir.go (Function.parsePragmas()) and used in compiler.go - probably in parseFunc. The procedure is something like:

  kind := llvm.AttributeKindID("signal")
  attr := c.ctx.CreateEnumAttribute(kind, 0)
  frame.fn.fn.llvmFn.AddFunctionAttr(attr)

  This attribute is necessary so that the compiler knows it must save all used registers and restore them before returning so that the currently running function isn't affected. Cortex-M does the equivalent in hardware so it doesn't need such an attribute.

Otherwise I'll probably implement this the next time I'm working on TinyGo.

[aykevl]

I have implemented support for autogenerated interrupt vectors in b963831, which is the first step towards interrupts on AVR. This also means the AVR support isn't as tightly tied to the atmega328p as before.

Turns out that adding the signal attribute isn't as easy as I originally expected. I'm currently investigating how to do that - the code above is not correct. Note: this attribute must be added to instruct LLVM to save and restore all registers used and generate a reti instruction at the end so the function can be safely called from an interrupt.

[aykevl]

Found it, here is a branch you can try: https://github.com/aykevl/tinygo/tree/avr-interrupts

I haven't tested it on the device but the generated assembly looks good. If you can confirm it works, I can merge it.

You can use it like this:

//go:interrupt INT0_vect
func handleINT0() {
    // ...
}

Of course, for communication between the ISR and main code, you need to use a volatile variable. This is currently supported using a hack: any type named __volatile is treated as volatile (see for example https://github.com/aykevl/tinygo/blob/master/src/runtime/runtime_nrf.go#L84). At some point I'd like to replace this with a pragma or even with channels (a goroutine receiving from a channel is compiled to an interrupt handler).

[deadprogram]

That looks very exciting. I will start playing with it, and let you know how it goes. Thanks!

[deadprogram]

I tried to define the following interrupt handler in machine_avr.go and then run the serial.go example:

//go:interrupt USART_RX_vect
func handleUSART_RX() {
	// Read UDR register to reset flag
	data := *avr.UDR0

	// echo back
	*avr.UDR0 = avr.RegValue(data)
}

When compiling with this branch, I received the following error:

$ make flash-serial TARGET=arduino
go build -o build/tgo -i .
./build/tgo build -target arduino -size=short -o build/serial.elf examples/serial
/tmp/ccVwfZM0.o: In function `__vector_default':
(.text.__vector_default+0x0): multiple definition of `__vector_default'
/tmp/ccrbdbOX.o:(.text.__vector_default+0x0): first defined here
collect2: error: ld returned 1 exit status
error: exit status 1
Makefile:85: recipe for target 'build/serial.elf' failed
make: *** [build/serial.elf] Error 1

[aykevl]

Took me a while to realize what was going on here.
The fix for the master branch (which moves __vector_default) is relevant here as well, so should be merged. Without it, and with src/device/avr generated from the master branch, __vector_default is defined twice.

At least that's my best guess.

[deadprogram]

After merging master into this branch I get a different error:

go build -o build/tgo -i .
# github.com/aykevl/tinygo/compiler
compiler/compiler.go:1138:50: c.triple undefined (type *Compiler has no field or method triple)
Makefile:76: recipe for target 'build/tgo' failed
make: *** [build/tgo] Error 2

[aykevl]

Ah, that's a different conflict due to some refactoring I did yesterday (cleaning up some code to make it easier to test). I've merged master myself and added a fix.

[aykevl]

You see, this compiler is still rather in flux ;) and I'm doing these changes now so they don't need to be done later when things get even more complicated.

[deadprogram]

Absolutely, yes!

[deadprogram]

OK thanks for the update. The code now compiles, but does not work as expected. Hard to tell what is wrong, how can I tell if my interrupt handler is being called? Also does my syntax from above look correct to you?

[aykevl]

Copied your code into examples/serial and disassembled the output:

make build/serial.elf TARGET=arduino
avr-objdump -Dz build/serial.elf | less

The ISR handler itself looks okay (ends in reti and saves all registers it uses):

00000108 <__vector_USART_RX>:
 108:   0f 92           push    r0
 10a:   1f 92           push    r1
 10c:   0f b6           in      r0, 0x3f        ; 63
 10e:   0f 92           push    r0
 110:   00 24           eor     r0, r0
 112:   8f 93           push    r24
 114:   80 91 c6 00     lds     r24, 0x00C6     ; 0x8000c6 <__flash_size+0x7f80c6>
 118:   80 93 c6 00     sts     0x00C6, r24     ; 0x8000c6 <__flash_size+0x7f80c6>
 11c:   8f 91           pop     r24
 11e:   0f 90           pop     r0
 120:   0f be           out     0x3f, r0        ; 63
 122:   1f 90           pop     r1
 124:   0f 90           pop     r0
 126:   18 95           reti

The vector also looks okay to me (it lists __vector_USART_RX):

Disassembly of section .text:

00000000 <__vector_RESET-0x68>:
   0:   0c 94 34 00     jmp     0x68    ; 0x68 <__vector_RESET>
   4:   0c 94 80 00     jmp     0x100   ; 0x100 <__vector_default>
   8:   0c 94 80 00     jmp     0x100   ; 0x100 <__vector_default>
   c:   0c 94 80 00     jmp     0x100   ; 0x100 <__vector_default>
  10:   0c 94 80 00     jmp     0x100   ; 0x100 <__vector_default>
  14:   0c 94 80 00     jmp     0x100   ; 0x100 <__vector_default>
  18:   0c 94 72 00     jmp     0xe4    ; 0xe4 <__vector_WDT>
  1c:   0c 94 80 00     jmp     0x100   ; 0x100 <__vector_default>
  20:   0c 94 80 00     jmp     0x100   ; 0x100 <__vector_default>
  24:   0c 94 80 00     jmp     0x100   ; 0x100 <__vector_default>
  28:   0c 94 80 00     jmp     0x100   ; 0x100 <__vector_default>
  2c:   0c 94 80 00     jmp     0x100   ; 0x100 <__vector_default>
  30:   0c 94 80 00     jmp     0x100   ; 0x100 <__vector_default>
  34:   0c 94 80 00     jmp     0x100   ; 0x100 <__vector_default>
  38:   0c 94 80 00     jmp     0x100   ; 0x100 <__vector_default>
  3c:   0c 94 80 00     jmp     0x100   ; 0x100 <__vector_default>
  40:   0c 94 80 00     jmp     0x100   ; 0x100 <__vector_default>
  44:   0c 94 80 00     jmp     0x100   ; 0x100 <__vector_default>
  48:   0c 94 84 00     jmp     0x108   ; 0x108 <__vector_USART_RX>
  4c:   0c 94 80 00     jmp     0x100   ; 0x100 <__vector_default>
  50:   0c 94 80 00     jmp     0x100   ; 0x100 <__vector_default>
  54:   0c 94 80 00     jmp     0x100   ; 0x100 <__vector_default>
  58:   0c 94 80 00     jmp     0x100   ; 0x100 <__vector_default>
  5c:   0c 94 80 00     jmp     0x100   ; 0x100 <__vector_default>
  60:   0c 94 80 00     jmp     0x100   ; 0x100 <__vector_default>
  64:   0c 94 80 00     jmp     0x100   ; 0x100 <__vector_default>

[aykevl]

Something that should work everywhere: enable/disable a GPIO pin. That's an easy way to detect whether the interrupt gets called at all.

[aykevl]

I've added the interrupt support to master to make it easier to work on. Disassembly looks good to me so I'm expecting it to work...

[deadprogram]

Just discovered that if I define my interrupt handler in machine_avr.go it does not get called. However, if I define it in runtime_avr.go it is called. I doubt that is expected behavior?

[deadprogram]

Good news is it was called!

[aykevl]

I've seen this before and it was because the SimpleDCE pass removed the external function when it shouldn't have removed it. But that should have been fixed. I tested it (by placing an interrupt handler in machine_avr.go) and the interrupt handler correctly shows up in the disassembly.
If this is still a problem, can you send me this file and/or the .elf file?

./build/tgo -o build/<example>.ll -target arduino examples/<example>

[deadprogram]

Yes, still a problem. Here are the .ll and .elf files.
tinygo-interrupt.zip

[aykevl]

Are you on master? (Just to be sure)

[deadprogram]

Yes, on master.

[aykevl]

This is... weird.
I have managed to reproduce it with examples/serial, but not with examples/blinkm.

[deadprogram]

👻

[aykevl]

Aaaah I think I got it.
examples/blinkm imports machine, examples/serial does not. Of course, if the package is not imported, the function won't be defined.
Could that be the case for you?

[deadprogram]

That is exactly it. My go vet or something removed the unused package from the imports, and I did not pay any attention. Verifying that now...

[deadprogram]

Confirmed. Good catch on that! OK so the code is working as expected.

My next question is, how can I use __volatile with a fixed array of bytes to serve as a ring buffer for the UART i/o? I want to be able to buffer the RX data to implement the UART Reader interface.

[aykevl]

I think if you use this it should work - every element of the array is of type __volatile so should be marked volatile in LLVM.

type __volatile uint8

var ringbuffer [20]__volatile
var head __volatile
var tail __volatile
// etc

You might also get away with only using __volatile for head + tail and leave it for the buffer, but this is not guaranteed to work.

Ideally I want to remove this __volatile hack as it's really inconvenient. The end goal is something like this:

//go:volatile
type ringbuffer struct {
    buf [20]byte
    head uint8 
    tail uint8
}

// NOTE: does not work yet
var rx ringbuffer

[deadprogram]

The current __volatile syntax works as expected. Now closing this issue, as both the original and the spin-off questions have been answered. Thank you!