transform: PtrToInt and IntToPtr do not escape

[ysoldak]

This hints compiler that constructs like uintptr(unsafe.Pointer(&x)) do not escape. Handled by llvm.PtrToInt case.
(I've also added llvm.IntToPtr, for completeness, but I'm not sure what code matches it. Feel free to reject.)

Disclaimer

This change may very well be completely wrong; can such constructs escape?
Hence a draft, merely a conversation starter.

Context

I2C implementations on RP2040 and NRF52 behave differently regarding allocation behavior.
NRF52 implementation escapes to heap while RP2040 does not.
The issue was tracked down to lines 52 and 61 of this file: https://github.com/tinygo-org/tinygo/blob/release/src/machine/machine_nrf528xx.go

Reproducer

$ tinygo build -target=feather-nrf52840 -print-allocs=main -o test.uf2 ./src/examples/i2c-target
.../tinygo/src/examples/i2c-target/main.go:101:13: object allocated on the heap: escapes at line 105
.../tinygo/src/examples/i2c-target/main.go:77:43: object allocated on the heap: escapes at line 77
.../tinygo/src/examples/i2c-target/main.go:76:12: object allocated on the heap: escapes at line 77
.../tinygo/src/examples/i2c-target/main.go:66:43: object allocated on the heap: escapes at line 66
.../tinygo/src/examples/i2c-target/main.go:57:43: object allocated on the heap: escapes at line 57
.../tinygo/src/examples/i2c-target/main.go:56:13: object allocated on the heap: escapes at line 57

And this does not escape at all

$ tinygo build -target=nano-rp2040 -print-allocs=main -o test.uf2 ./src/examples/i2c-target

After this change both do not escape anymore.
I was over-optimistic and probably tired, it still escapes in NRF52 case, but I don't understand why.

Thanks @dgryski for pointing at allocs.go

[ysoldak]

In machine package we have many cases of uintptr(unsafe use.
See https://github.com/search?q=repo%3Atinygo-org%2Ftinygo%20uintptr(unsafe%20path%3Asrc%2Fmachine&type=code

Each of such cases potentially unnecessarily allocates.

[dgryski]

From a conservation on Slack:

I think we need a bit more; for example if you have a pointer conversion between two pointer types, we want to make sure the first one "escapes" if the second pointer "escapes" too.
That's probably why the official unsafe rules require all pointer conversions to happen in a single expression, so there's no requirement to track "does this int eventually become a pointer".
This code pattern with IntToPtr will happen with code predating https://pkg.go.dev/unsafe#Add.

[eliasnaur]

As far as I can read the NRF code i2c.Bus.TXD.PTR.Set(uint32(uintptr(unsafe.Pointer(&w[0])))) fundamentally does escape the pointer, no? I realize that Tx does wait for the DMA transfer to complete, but the TinyGo compiler doesn't know that.

In other words, I can't see how this optimization can be done even with perfect tracking of the uintptr value.

As a side note, I believe code such as the NRF i2c.Tx technically needs a runtime.Pinner to keep the w and r buffers pinned during the DMA transfer. Because of #4072 and because TinyGo doesn't move pointers, a runtime.KeepAlive is probably enough.

[aykevl]

As far as I can read the NRF code i2c.Bus.TXD.PTR.Set(uint32(uintptr(unsafe.Pointer(&w[0])))) fundamentally does escape the pointer, no? I realize that Tx does wait for the DMA transfer to complete, but the TinyGo compiler doesn't know that.

In other words, I can't see how this optimization can be done even with perfect tracking of the uintptr value.

Agreed. The value does escape here.

We could possibly do something tricky like telling the compiler "please keep the value alive until this point (after the DMA transfer completed), but don't consider this pointer escaped". That would be correct, and also avoid the heap allocation. But I'm not entirely sure how to do that - perhaps some tricks with inline assembly can be used.

EDIT: actually, runtime.KeepAlive does exactly what I describe above. And it is already implemented in TinyGo. The only thing missing is a way to tell the compiler "this PtrToInt doesn't escape".

[eliasnaur]

As far as I can read the NRF code i2c.Bus.TXD.PTR.Set(uint32(uintptr(unsafe.Pointer(&w[0])))) fundamentally does escape the pointer, no? I realize that Tx does wait for the DMA transfer to complete, but the TinyGo compiler doesn't know that.
In other words, I can't see how this optimization can be done even with perfect tracking of the uintptr value.

Agreed. The value does escape here.

We could possibly do something tricky like telling the compiler "please keep the value alive until this point (after the DMA transfer completed), but don't consider this pointer escaped". That would be correct, and also avoid the heap allocation. But I'm not entirely sure how to do that - perhaps some tricks with inline assembly can be used.

Isn't that what runtime.Pinner is for? That is, fixing #4072 will allow low-level code to extend a pointer's lifetime, but shouldn't interfere with escape analysis. Longer term, runtime.Pinner will mark a pointer pinned for a future moving garbage collector.

[eliasnaur]

EDIT: actually, runtime.KeepAlive does exactly what I describe above. And it is already implemented in TinyGo. The only thing missing is a way to tell the compiler "this PtrToInt doesn't escape".

Pedantically, runtime.KeepAlive doesn't say that the pointer address should be held constant, only that finalizers won't run. DMA requires pinned addresses, as provided by runtime.Pinner.

[aykevl]

Isn't that what runtime.Pinner is for?

No, that's to avoid moving the object (which doesn't currently happen and realistically is unlikely to happen in the future). A quick look at runtime.Pinner seems to show that the object will likely escape.
What we actually want is runtime.KeepAlive. See the edit to my previous comment.

[aykevl]

Pedantically, runtime.KeepAlive doesn't say that the pointer address should be held constant, only that finalizers won't run. DMA requires pinned addresses, as provided by runtime.Pinner.

Well you are technically correct. Do note that runtime.KeepAlive also promises to keep the object alive, it just says nothing about the address:

KeepAlive marks its argument as currently reachable. This ensures that the object is not freed, and its finalizer is not run, before the point in the program where KeepAlive is called.

In TinyGo, I think it is unlikely we'll ever have a moving garbage collector that would require pinning. Not just because of DMA, but also because it means interrupts can't run while the GC moves the heap.

[eliasnaur]

which doesn't currently happen and realistically is unlikely to happen in the future

I've seen this comment before, but I'd like to say I find the current garbage collector too unreliable for dynamic allocation because of memory fragmentation. I frequently run into out-of-memory issues before even half of available memory (512kB) has been filled because of fragmentation. Coupled with the escape analyzer missing key allocations I find myself spending significant effort (and API contortions) to avoid allocations altogether.

I don't see any other way of fixing fragmentation and making the GC reliable than making it moving.

[eliasnaur]

In TinyGo, I think it is unlikely we'll ever have a moving garbage collector that would require pinning. Not just because of DMA, but also because it means interrupts can't run while the GC moves the heap.

DMA code as well as interrupts can mark objects pinned, no? I realize this is not done today, but I don't see a fundamental reason that's impossible.