Skip to content

machine: make sure DMA buffers do not escape unnecessarily #4930

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from
Open

machine: make sure DMA buffers do not escape unnecessarily #4930

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions compiler/intrinsics.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ func (b *builder) defineIntrinsicFunction() {
b.createStackSaveImpl()
case name == "runtime.KeepAlive":
b.createKeepAliveImpl()
case name == "machine.keepAliveNoEscape":
b.createMachineKeepAliveImpl()
case strings.HasPrefix(name, "runtime/volatile.Load"):
b.createVolatileLoad()
case strings.HasPrefix(name, "runtime/volatile.Store"):
Expand Down Expand Up @@ -121,6 +123,20 @@ func (b *builder) createKeepAliveImpl() {
b.CreateRetVoid()
}

// Implement machine.keepAliveNoEscape, which makes sure the compiler keeps the
// pointer parameter alive until this point (for GC).
func (b *builder) createMachineKeepAliveImpl() {
b.createFunctionStart(true)
pointerValue := b.getValue(b.fn.Params[0], getPos(b.fn))

// See createKeepAliveImpl for details.
asmType := llvm.FunctionType(b.ctx.VoidType(), []llvm.Type{b.dataPtrType}, false)
asmFn := llvm.InlineAsm(asmType, "", "r", true, false, 0, false)
b.createCall(asmType, asmFn, []llvm.Value{pointerValue}, "")

b.CreateRetVoid()
}

var mathToLLVMMapping = map[string]string{
"math.Ceil": "llvm.ceil.f64",
"math.Exp": "llvm.exp.f64",
Expand Down
2 changes: 2 additions & 0 deletions compiler/symbol.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,8 @@ func (c *compilerContext) getFunction(fn *ssa.Function) (llvm.Type, llvm.Value)
llvmFn.AddFunctionAttr(c.ctx.CreateEnumAttribute(llvm.AttributeKindID("noreturn"), 0))
case "internal/abi.NoEscape":
llvmFn.AddAttributeAtIndex(1, c.ctx.CreateEnumAttribute(llvm.AttributeKindID("nocapture"), 0))
case "machine.keepAliveNoEscape", "machine.unsafeNoEscape":
llvmFn.AddAttributeAtIndex(1, c.ctx.CreateEnumAttribute(llvm.AttributeKindID("nocapture"), 0))
case "runtime.alloc":
// Tell the optimizer that runtime.alloc is an allocator, meaning that it
// returns values that are never null and never alias to an existing value.
Expand Down
32 changes: 31 additions & 1 deletion src/machine/machine.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
package machine

import "errors"
import (
"errors"
"unsafe"
)

var (
ErrTimeoutRNG = errors.New("machine: RNG Timeout")
Expand Down Expand Up @@ -62,3 +65,30 @@ func (p Pin) Low() {
type ADC struct {
Pin Pin
}

// Convert the pointer to a uintptr, to be used for memory I/O (DMA for
// example). It also means the pointer is "gone" as far as the compiler is
// concerned, and a GC cycle might deallocate the object. To prevent this from
// happening, also call keepAliveNoEscape at a point after the address isn't
// accessed anymore by the hardware.
// The only exception is if the pointer is accessed later in a volatile way
// (volatile read/write), which also forces the value to stay alive until that
// point.
//
// This function is treated specially by the compiler to mark the 'ptr'
// parameter as not escaping.
//
// TODO: this function should eventually be replaced with the proposed ptrtoaddr
// instruction in LLVM. See:
// https://discourse.llvm.org/t/clarifiying-the-semantics-of-ptrtoint/83987/10
// https://github.com/llvm/llvm-project/pull/139357
func unsafeNoEscape(ptr unsafe.Pointer) uintptr {
return uintptr(ptr)
}

// Make sure the given pointer stays alive until this point. This is similar to
// runtime.KeepAlive, with the difference that it won't let the pointer escape.
// This is typically used together with unsafeNoEscape.
//
// This is a compiler intrinsic.
func keepAliveNoEscape(ptr unsafe.Pointer)
Comment on lines +69 to +94
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is, as you say, hacky and won't help external drivers that don't have access to the unexported unsafeNoEscape and keepAliveNoEscape.

Can't runtime.KeepAlive be made to not move the allocation to the heap in TinyGo? If so, that leaves unsafeNoEscape. I suggest the more drastical solution of accepting #4889 that relaxes uintptr(unsafe.Pointer(&x)) to not escape. My reasoning is that for something like DMA, you need something like runtime.KeepAlive to keep the pointer alive anyway (it's not enough for the pointer to be kept alive at the point of conversion to uintptr).

That way, all code can use the same tricks to avoid allocations.

Copy link
Member Author

@aykevl aykevl Jun 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is, as you say, hacky and won't help external drivers that don't have access to the unexported unsafeNoEscape and keepAliveNoEscape.

I intentionally kept them unexported. Drivers (like the tinygo.org/x/drivers/* packages) won't need it as long as they don't access these hardware registers directly - which is the vast majority. If there are drivers which really need these functions, we can export these functions.

Can't runtime.KeepAlive be made to not move the allocation to the heap in TinyGo?

Yes, but that won't help the machine package due to a circular dependency (machine -> runtime -> machine).

I suggest the more drastical solution of accepting #4889 that relaxes uintptr(unsafe.Pointer(&x)) to not escape.

The only way to implement that at the moment is to let the compiler do something exactly like unsafeNoEscape: insert a call at the point of the cast. I much prefer to keep this explicit and keep the default (escaping) behavior for these casts. In the future there might be ptrtoaddr but not today.

My reasoning is that for something like DMA, you need something like runtime.KeepAlive to keep the pointer alive anyway (it's not enough for the pointer to be kept alive at the point of conversion to uintptr).

Yeah that's the goal of this PR.

25 changes: 21 additions & 4 deletions src/machine/machine_nrf528xx.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ func (i2c *I2C) Tx(addr uint16, w, r []byte) (err error) {

// Configure for a single shot to perform both write and read (as applicable)
if len(w) != 0 {
i2c.Bus.TXD.PTR.Set(uint32(uintptr(unsafe.Pointer(&w[0]))))
i2c.Bus.TXD.PTR.Set(uint32(unsafeNoEscape(unsafe.Pointer(&w[0]))))
i2c.Bus.TXD.MAXCNT.Set(uint32(len(w)))

// If no read, immediately signal stop after TX
Expand All @@ -58,7 +58,7 @@ func (i2c *I2C) Tx(addr uint16, w, r []byte) (err error) {
}
}
if len(r) != 0 {
i2c.Bus.RXD.PTR.Set(uint32(uintptr(unsafe.Pointer(&r[0]))))
i2c.Bus.RXD.PTR.Set(uint32(unsafeNoEscape(unsafe.Pointer(&r[0]))))
i2c.Bus.RXD.MAXCNT.Set(uint32(len(r)))

// Auto-start Rx after Tx and Stop after Rx
Expand Down Expand Up @@ -89,6 +89,15 @@ func (i2c *I2C) Tx(addr uint16, w, r []byte) (err error) {
}
}

// Make sure the w and r buffers stay alive until this point, so they won't
// be garbage collected while the buffers are used by the hardware.
if len(w) > 0 {
keepAliveNoEscape(unsafe.Pointer(&w[0]))
}
if len(r) > 0 {
keepAliveNoEscape(unsafe.Pointer(&r[0]))
}

return
}

Expand Down Expand Up @@ -117,7 +126,7 @@ func (i2c *I2C) Listen(addr uint8) error {
//
// For request events, the caller MUST call `Reply` to avoid hanging the i2c bus indefinitely.
func (i2c *I2C) WaitForEvent(buf []byte) (evt I2CTargetEvent, count int, err error) {
i2c.BusT.RXD.PTR.Set(uint32(uintptr(unsafe.Pointer(&buf[0]))))
i2c.BusT.RXD.PTR.Set(uint32(unsafeNoEscape(unsafe.Pointer(&buf[0]))))
i2c.BusT.RXD.MAXCNT.Set(uint32(len(buf)))

i2c.BusT.TASKS_PREPARERX.Set(nrf.TWIS_TASKS_PREPARERX_TASKS_PREPARERX_Trigger)
Expand All @@ -134,6 +143,10 @@ func (i2c *I2C) WaitForEvent(buf []byte) (evt I2CTargetEvent, count int, err err
}
}

// Make sure buf stays alive until this point, so it won't be garbage
// collected while it is used by the hardware.
keepAliveNoEscape(unsafe.Pointer(&buf[0]))

count = 0
evt = I2CFinish
err = nil
Expand Down Expand Up @@ -163,7 +176,7 @@ func (i2c *I2C) WaitForEvent(buf []byte) (evt I2CTargetEvent, count int, err err

// Reply supplies the response data the controller.
func (i2c *I2C) Reply(buf []byte) error {
i2c.BusT.TXD.PTR.Set(uint32(uintptr(unsafe.Pointer(&buf[0]))))
i2c.BusT.TXD.PTR.Set(uint32(unsafeNoEscape(unsafe.Pointer(&buf[0]))))
i2c.BusT.TXD.MAXCNT.Set(uint32(len(buf)))

i2c.BusT.EVENTS_STOPPED.Set(0)
Expand All @@ -180,6 +193,10 @@ func (i2c *I2C) Reply(buf []byte) error {
}
}

// Make sure the buffer stays alive until this point, so it won't be garbage
// collected while it is used by the hardware.
keepAliveNoEscape(unsafe.Pointer(&buf[0]))

i2c.BusT.EVENTS_STOPPED.Set(0)

return nil
Expand Down
17 changes: 14 additions & 3 deletions src/machine/machine_nrf52xxx.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,9 @@ func (a *ADC) Get() uint16 {
nrf.SAADC.CH[0].PSELP.Set(pwmPin)

// Destination for sample result.
nrf.SAADC.RESULT.PTR.Set(uint32(uintptr(unsafe.Pointer(&rawValue))))
// Note: rawValue doesn't need to be kept alive for the GC, since the
// volatile read later will force it to stay alive.
nrf.SAADC.RESULT.PTR.Set(uint32(unsafeNoEscape(unsafe.Pointer(&rawValue))))
nrf.SAADC.RESULT.MAXCNT.Set(1) // One sample

// Start tasks.
Expand Down Expand Up @@ -314,7 +316,7 @@ func (spi *SPI) Tx(w, r []byte) error {
if nr > 255 {
nr = 255
}
spi.Bus.RXD.PTR.Set(uint32(uintptr(unsafe.Pointer(&r[0]))))
spi.Bus.RXD.PTR.Set(uint32(unsafeNoEscape(unsafe.Pointer(&r[0]))))
r = r[nr:]
}
spi.Bus.RXD.MAXCNT.Set(nr)
Expand All @@ -325,7 +327,7 @@ func (spi *SPI) Tx(w, r []byte) error {
if nw > 255 {
nw = 255
}
spi.Bus.TXD.PTR.Set(uint32(uintptr(unsafe.Pointer(&w[0]))))
spi.Bus.TXD.PTR.Set(uint32(unsafeNoEscape(unsafe.Pointer(&w[0]))))
w = w[nw:]
}
spi.Bus.TXD.MAXCNT.Set(nw)
Expand All @@ -339,6 +341,15 @@ func (spi *SPI) Tx(w, r []byte) error {
spi.Bus.EVENTS_END.Set(0)
}

// Make sure the w and r buffers stay alive for the GC until this point,
// since they are used by the hardware but not otherwise visible.
if len(r) != 0 {
keepAliveNoEscape(unsafe.Pointer(&r[0]))
}
if len(w) != 0 {
keepAliveNoEscape(unsafe.Pointer(&w[0]))
}

return nil
}

Expand Down
7 changes: 6 additions & 1 deletion src/machine/machine_rp2_spi.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,7 @@ func (spi *SPI) tx(tx []byte) error {
// - set data size to single bytes
// - set the DREQ so that the DMA will fill the SPI FIFO as needed
// - start the transfer
ch.READ_ADDR.Set(uint32(uintptr(unsafe.Pointer(&tx[0]))))
ch.READ_ADDR.Set(uint32(unsafeNoEscape(unsafe.Pointer(&tx[0]))))
ch.WRITE_ADDR.Set(uint32(uintptr(unsafe.Pointer(&spi.Bus.SSPDR))))
ch.TRANS_COUNT.Set(uint32(len(tx)))
ch.CTRL_TRIG.Set(rp.DMA_CH0_CTRL_TRIG_INCR_READ |
Expand All @@ -328,6 +328,11 @@ func (spi *SPI) tx(tx []byte) error {
for ch.CTRL_TRIG.Get()&rp.DMA_CH0_CTRL_TRIG_BUSY != 0 {
}

// Make sure the read buffer stays alive until this point (in the unlikely
// case the tx slice wasn't read after this function returns and a GC cycle
// happened inbetween).
keepAliveNoEscape(unsafe.Pointer(&tx[0]))

// We didn't read any result values, which means the RX FIFO has likely
// overflown. We have to clean up this mess now.

Expand Down