Skip to content

Commit e2178ce

Browse files
niaowdeadprogram
authored andcommitted
compiler: work around AVR atomics bugs
The AVR backend has several critical atomics bugs. This change invokes libcalls for all atomic operations on AVR. Now `testdata/atomic.go` compiles and runs correctly.
1 parent b2ef729 commit e2178ce

File tree

3 files changed

+53
-1
lines changed

3 files changed

+53
-1
lines changed

compiler/atomic.go

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package compiler
22

33
import (
4+
"fmt"
45
"strings"
56

67
"golang.org/x/tools/go/ssa"
@@ -16,6 +17,20 @@ func (b *builder) createAtomicOp(call *ssa.CallCommon) (llvm.Value, bool) {
1617
case "AddInt32", "AddInt64", "AddUint32", "AddUint64", "AddUintptr":
1718
ptr := b.getValue(call.Args[0])
1819
val := b.getValue(call.Args[1])
20+
if strings.HasPrefix(b.Triple, "avr") {
21+
// AtomicRMW does not work on AVR as intended:
22+
// - There are some register allocation issues (fixed by https://reviews.llvm.org/D97127 which is not yet in a usable LLVM release)
23+
// - The result is the new value instead of the old value
24+
vType := val.Type()
25+
name := fmt.Sprintf("__sync_fetch_and_add_%d", vType.IntTypeWidth()/8)
26+
fn := b.mod.NamedFunction(name)
27+
if fn.IsNil() {
28+
fn = llvm.AddFunction(b.mod, name, llvm.FunctionType(vType, []llvm.Type{ptr.Type(), vType}, false))
29+
}
30+
oldVal := b.createCall(fn, []llvm.Value{ptr, val}, "")
31+
// Return the new value, not the original value returned.
32+
return b.CreateAdd(oldVal, val, ""), true
33+
}
1934
oldVal := b.CreateAtomicRMW(llvm.AtomicRMWBinOpAdd, ptr, val, llvm.AtomicOrderingSequentiallyConsistent, true)
2035
// Return the new value, not the original value returned by atomicrmw.
2136
return b.CreateAdd(oldVal, val, ""), true
@@ -73,6 +88,23 @@ func (b *builder) createAtomicOp(call *ssa.CallCommon) (llvm.Value, bool) {
7388
case "StoreInt32", "StoreInt64", "StoreUint32", "StoreUint64", "StoreUintptr", "StorePointer":
7489
ptr := b.getValue(call.Args[0])
7590
val := b.getValue(call.Args[1])
91+
if strings.HasPrefix(b.Triple, "avr") {
92+
// SelectionDAGBuilder is currently missing the "are unaligned atomics allowed" check for stores.
93+
vType := val.Type()
94+
isPointer := vType.TypeKind() == llvm.PointerTypeKind
95+
if isPointer {
96+
// libcalls only supports integers, so cast to an integer.
97+
vType = b.uintptrType
98+
val = b.CreatePtrToInt(val, vType, "")
99+
ptr = b.CreateBitCast(ptr, llvm.PointerType(vType, 0), "")
100+
}
101+
name := fmt.Sprintf("__atomic_store_%d", vType.IntTypeWidth()/8)
102+
fn := b.mod.NamedFunction(name)
103+
if fn.IsNil() {
104+
fn = llvm.AddFunction(b.mod, name, llvm.FunctionType(vType, []llvm.Type{ptr.Type(), vType, b.uintptrType}, false))
105+
}
106+
return b.createCall(fn, []llvm.Value{ptr, val, llvm.ConstInt(b.uintptrType, 5, false)}, ""), true
107+
}
76108
store := b.CreateStore(val, ptr)
77109
store.SetOrdering(llvm.AtomicOrderingSequentiallyConsistent)
78110
store.SetAlignment(b.targetData.PrefTypeAlignment(val.Type())) // required

main_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,8 @@ func TestBuild(t *testing.T) {
140140
for _, t := range tests {
141141
switch t {
142142
case "atomic.go":
143-
// Not supported due to unaligned atomic accesses.
143+
// Requires GCC 11.2.0 or above for interface comparison.
144+
// https://github.com/gcc-mirror/gcc/commit/f30dd607669212de135dec1f1d8a93b8954c327c
144145

145146
case "reflect.go":
146147
// Reflect tests do not work due to type code issues.

src/runtime/arch_avr.go

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
1+
//go:build avr
12
// +build avr
23

34
package runtime
45

6+
import "runtime/interrupt"
7+
58
const GOARCH = "arm" // avr pretends to be arm
69

710
// The bitness of the CPU (e.g. 8, 32, 64).
@@ -16,3 +19,19 @@ func align(ptr uintptr) uintptr {
1619
func getCurrentStackPointer() uintptr {
1720
return uintptr(stacksave())
1821
}
22+
23+
// The safest thing to do here would just be to disable interrupts for
24+
// procPin/procUnpin. Note that a global variable is safe in this case, as any
25+
// access to procPinnedMask will happen with interrupts disabled.
26+
27+
var procPinnedMask interrupt.State
28+
29+
//go:linkname procPin sync/atomic.runtime_procPin
30+
func procPin() {
31+
procPinnedMask = interrupt.Disable()
32+
}
33+
34+
//go:linkname procUnpin sync/atomic.runtime_procUnpin
35+
func procUnpin() {
36+
interrupt.Restore(procPinnedMask)
37+
}

0 commit comments

Comments
 (0)