compiler: implement deferred calls to sync/atomic.*

[skabbes]

Fixes: #2652 , #2805
And is part of a larger effort to get Protobufs working with tinygo. #2667

Previously discussed here, and here.

Problem
Deferrred calls to the sync/atomic package would fail with a linker error, since those functions don't really exist in the LLVM representation.

package main

import (
	"sync/atomic"
)

var global uint32

func foo() {
	defer atomic.StoreUint32(&global, 2) // the offending line
	atomic.StoreUint32(&global, 1)
	println(atomic.LoadUint32(&global))
}

func main() {
	foo()
	println(atomic.LoadUint32(&global))
}

Solution
After some discussion in the referenced issues, the solution I pursued is to share the previous rewrite code with createRunDefers. (In effect the deferred calls to atomic.* are inlined in exactly the same way).

Next Steps
The amount of code changed is small, and easy to understand, and given this has never worked before - I am fairly confident it won't break anything. The tests 'prove' it works, but given the "run defers" code seems particularly hairy (I would love a second set of eyes on it).

@aykevl this is a slight modification of your original idea here, do you mind taking a look?

Please feel free to hack away at my code if you'd find that faster than coordinating the work with me over comments.

[skabbes]

@aykevl I added tests for each individual call to the sync/atomic.* package. Would you be willing to review this? I think it is pretty straightforward, as it is leveraging the existing code for rewriting atomics (no changes there).

The build failures seem completely unrelated, maybe due to go 1.18, or some CI configuration changes to windows.

[skabbes]

I think windows is failing due to an upstream scoop issue:
ScoopInstaller/Scoop#4917

[skabbes]

I think #2853 will resolve build issues 🤞

[deadprogram]

@skabbes please rebase against dev now to see if it fixes the build on this branch. Thanks!

[skabbes]

@deadprogram all green!

[skabbes]

FYI - as deadprogram has requested of me on other PRs - I can clean up / squash / whatever the commits once we have completed a code review.

[aykevl]

While this probably works fine, I think there is a much more elegant way: by defining the functions (like sync/atomic.AddInt32) directly instead of changing all the calls to it. This will automatically make deferred calls work correctly without any special cases (and avoid the hairiness of it all).

Here is how to do that. First you can detect the special sync/atomic functions here:

diff --git a/compiler/compiler.go b/compiler/compiler.go
index dd7a8ea3..6981fc06 100644
--- a/compiler/compiler.go
+++ b/compiler/compiler.go
@@ -773,6 +773,9 @@ func (c *compilerContext) createPackage(irbuilder llvm.Builder, pkg *ssa.Package
                        // Create the function definition.
                        b := newBuilder(c, irbuilder, member)
                        if member.Blocks == nil {
+                               if member.Pkg.Pkg.Path() == "sync/atomic" && token.IsExported(member.Name()) {
+                                       b.implementAtomicOp()
+                               }
                                continue // external function
                        }
                        b.createFunction()

And then you can fill them with atomic instructions here:

func (b *builder) implementAtomicOp() {
	entry := b.ctx.AddBasicBlock(b.llvmFn, "entry")
	b.Builder.SetInsertPointAtEnd(entry)
	switch b.fn.Name() {
	case "AddInt32":
		ptr := b.llvmFn.Param(0)
		val := b.llvmFn.Param(1)
		oldVal := b.CreateAtomicRMW(llvm.AtomicRMWBinOpAdd, ptr, val, llvm.AtomicOrderingSequentiallyConsistent, true)
		// Return the new value, not the original value returned by atomicrmw.
		newVal := b.CreateAdd(oldVal, val, "")
		b.CreateRet(newVal)
	default:
		b.addError(b.fn.Pos(), "unknown atomic operation: "+b.fn.Name())
		b.CreateUnreachable()
	}
}

(This is not complete of course, it only implements sync/atomic.AddInt32 and doesn't implement the AVR special case).

[skabbes]

OK, thanks - I don't think I'll be able to get around to the changes. But maybe someone else can pick up the slack.

[aykevl]

@skabbes ok, I've implemented this as I described, see #2920.

[dgryski]

Closing as this was implemented in #2920