Skip to content

Commit 9890c76

Browse files
niaowaykevl
niaow
authored and
aykevl
committed
transform (func-lowering): remove specializations from function value lowering and fix lowering of a function value of an unimplemented type
Previously, the function value lowering pass had special cases for when there were 0 or 1 function implementations. However, the results of the pass were incorrect in both of these cases. This change removes the specializations and fixes the transformation. In the case that there was a single function implementation, the compiler emitted a select instruction to obtain the function pointer. This selected between null and the implementing function pointer. While this was technically correct, it failed to eliminate indirect function calls. This prevented discovery of these calls by the coroutine lowering pass, and caused async function calls to be passed through unlowered. As a result, the generated code had undefined behavior (usually resulting in a segfault). In the case of no function implementations, the lowering code was correct. However, the lowering code was not run. The discovery of function signatures was accomplished by scanning implementations, and when there were no implementations nothing was discovered or lowered. For maintainability reasons, I have removed both specializations rather than fixing them. This substantially simplifies the code, and reduces the amount of variation that we need to worry about for testing purposes. The IR now generated in the cases of 0 or 1 function implementations can be efficiently simplified by LLVM's optimization passes. Therefore, there should not be a substantial regression in terms of performance or machine code size.
1 parent 0a8bfc5 commit 9890c76

File tree

3 files changed

+108
-119
lines changed

3 files changed

+108
-119
lines changed

‎transform/func-lowering.go‎

Lines changed: 82 additions & 94 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ package transform
66
import (
77
"sort"
88
"strconv"
9+
"strings"
910

1011
"github.com/tinygo-org/tinygo/compiler/llvmutil"
1112
"tinygo.org/x/go-llvm"
@@ -55,17 +56,30 @@ func LowerFuncValues(mod llvm.Module) {
5556
funcValueWithSignaturePtr := llvm.PointerType(mod.GetTypeByName("runtime.funcValueWithSignature"), 0)
5657
signatures := map[string]*funcSignatureInfo{}
5758
for global := mod.FirstGlobal(); !global.IsNil(); global = llvm.NextGlobal(global) {
58-
if global.Type() != funcValueWithSignaturePtr {
59+
var sig, funcVal llvm.Value
60+
switch {
61+
case global.Type() == funcValueWithSignaturePtr:
62+
sig = llvm.ConstExtractValue(global.Initializer(), []uint32{1})
63+
funcVal = global
64+
case strings.HasPrefix(global.Name(), "reflect/types.type:func:{"):
65+
sig = global
66+
default:
5967
continue
6068
}
61-
sig := llvm.ConstExtractValue(global.Initializer(), []uint32{1})
69+
6270
name := sig.Name()
71+
var funcValueWithSignatures []llvm.Value
72+
if funcVal.IsNil() {
73+
funcValueWithSignatures = []llvm.Value{}
74+
} else {
75+
funcValueWithSignatures = []llvm.Value{funcVal}
76+
}
6377
if info, ok := signatures[name]; ok {
64-
info.funcValueWithSignatures = append(info.funcValueWithSignatures, global)
78+
info.funcValueWithSignatures = append(info.funcValueWithSignatures, funcValueWithSignatures...)
6579
} else {
6680
signatures[name] = &funcSignatureInfo{
6781
sig: sig,
68-
funcValueWithSignatures: []llvm.Value{global},
82+
funcValueWithSignatures: funcValueWithSignatures,
6983
}
7084
}
7185
}
@@ -123,95 +137,64 @@ func LowerFuncValues(mod llvm.Module) {
123137
panic("expected all call uses to be runtime.getFuncPtr")
124138
}
125139
funcID := getFuncPtrCall.Operand(1)
126-
switch len(functions) {
127-
case 0:
128-
// There are no functions used in a func value that implement
129-
// this signature. The only possible value is a nil value.
130-
for _, inttoptr := range getUses(getFuncPtrCall) {
131-
if inttoptr.IsAIntToPtrInst().IsNil() {
132-
panic("expected inttoptr")
133-
}
134-
nilptr := llvm.ConstPointerNull(inttoptr.Type())
135-
inttoptr.ReplaceAllUsesWith(nilptr)
136-
inttoptr.EraseFromParentAsInstruction()
140+
141+
// There are functions used in a func value that
142+
// implement this signature.
143+
// What we'll do is transform the following:
144+
// rawPtr := runtime.getFuncPtr(func.ptr)
145+
// if rawPtr == nil {
146+
// runtime.nilPanic()
147+
// }
148+
// result := rawPtr(...args, func.context)
149+
// into this:
150+
// if false {
151+
// runtime.nilPanic()
152+
// }
153+
// var result // Phi
154+
// switch fn.id {
155+
// case 0:
156+
// runtime.nilPanic()
157+
// case 1:
158+
// result = call first implementation...
159+
// case 2:
160+
// result = call second implementation...
161+
// default:
162+
// unreachable
163+
// }
164+
165+
// Remove some casts, checks, and the old call which we're going
166+
// to replace.
167+
for _, callIntPtr := range getUses(getFuncPtrCall) {
168+
if !callIntPtr.IsACallInst().IsNil() && callIntPtr.CalledValue().Name() == "internal/task.start" {
169+
// Special case for goroutine starts.
170+
addFuncLoweringSwitch(mod, builder, funcID, callIntPtr, func(funcPtr llvm.Value, params []llvm.Value) llvm.Value {
171+
i8ptrType := llvm.PointerType(ctx.Int8Type(), 0)
172+
calleeValue := builder.CreatePtrToInt(funcPtr, uintptrType, "")
173+
start := mod.NamedFunction("internal/task.start")
174+
builder.CreateCall(start, []llvm.Value{calleeValue, callIntPtr.Operand(1), llvm.Undef(i8ptrType), llvm.ConstNull(i8ptrType)}, "")
175+
return llvm.Value{} // void so no return value
176+
}, functions)
177+
callIntPtr.EraseFromParentAsInstruction()
178+
continue
137179
}
138-
getFuncPtrCall.EraseFromParentAsInstruction()
139-
case 1:
140-
// There is exactly one function with this signature that is
141-
// used in a func value. The func value itself can be either nil
142-
// or this one function.
143-
builder.SetInsertPointBefore(getFuncPtrCall)
144-
zero := llvm.ConstInt(uintptrType, 0, false)
145-
isnil := builder.CreateICmp(llvm.IntEQ, funcID, zero, "")
146-
funcPtrNil := llvm.ConstPointerNull(functions[0].funcPtr.Type())
147-
funcPtr := builder.CreateSelect(isnil, funcPtrNil, functions[0].funcPtr, "")
148-
for _, inttoptr := range getUses(getFuncPtrCall) {
149-
if inttoptr.IsAIntToPtrInst().IsNil() {
150-
panic("expected inttoptr")
151-
}
152-
inttoptr.ReplaceAllUsesWith(funcPtr)
153-
inttoptr.EraseFromParentAsInstruction()
180+
if callIntPtr.IsAIntToPtrInst().IsNil() {
181+
panic("expected inttoptr")
154182
}
155-
getFuncPtrCall.EraseFromParentAsInstruction()
156-
default:
157-
// There are multiple functions used in a func value that
158-
// implement this signature.
159-
// What we'll do is transform the following:
160-
// rawPtr := runtime.getFuncPtr(func.ptr)
161-
// if rawPtr == nil {
162-
// runtime.nilPanic()
163-
// }
164-
// result := rawPtr(...args, func.context)
165-
// into this:
166-
// if false {
167-
// runtime.nilPanic()
168-
// }
169-
// var result // Phi
170-
// switch fn.id {
171-
// case 0:
172-
// runtime.nilPanic()
173-
// case 1:
174-
// result = call first implementation...
175-
// case 2:
176-
// result = call second implementation...
177-
// default:
178-
// unreachable
179-
// }
180-
181-
// Remove some casts, checks, and the old call which we're going
182-
// to replace.
183-
for _, callIntPtr := range getUses(getFuncPtrCall) {
184-
if !callIntPtr.IsACallInst().IsNil() && callIntPtr.CalledValue().Name() == "internal/task.start" {
185-
// Special case for goroutine starts.
186-
addFuncLoweringSwitch(mod, builder, funcID, callIntPtr, func(funcPtr llvm.Value, params []llvm.Value) llvm.Value {
187-
i8ptrType := llvm.PointerType(ctx.Int8Type(), 0)
188-
calleeValue := builder.CreatePtrToInt(funcPtr, uintptrType, "")
189-
start := mod.NamedFunction("internal/task.start")
190-
builder.CreateCall(start, []llvm.Value{calleeValue, callIntPtr.Operand(1), llvm.Undef(i8ptrType), llvm.ConstNull(i8ptrType)}, "")
191-
return llvm.Value{} // void so no return value
183+
for _, ptrUse := range getUses(callIntPtr) {
184+
if !ptrUse.IsAICmpInst().IsNil() {
185+
ptrUse.ReplaceAllUsesWith(llvm.ConstInt(ctx.Int1Type(), 0, false))
186+
} else if !ptrUse.IsACallInst().IsNil() && ptrUse.CalledValue() == callIntPtr {
187+
addFuncLoweringSwitch(mod, builder, funcID, ptrUse, func(funcPtr llvm.Value, params []llvm.Value) llvm.Value {
188+
return builder.CreateCall(funcPtr, params, "")
192189
}, functions)
193-
callIntPtr.EraseFromParentAsInstruction()
194-
continue
195-
}
196-
if callIntPtr.IsAIntToPtrInst().IsNil() {
197-
panic("expected inttoptr")
198-
}
199-
for _, ptrUse := range getUses(callIntPtr) {
200-
if !ptrUse.IsAICmpInst().IsNil() {
201-
ptrUse.ReplaceAllUsesWith(llvm.ConstInt(ctx.Int1Type(), 0, false))
202-
} else if !ptrUse.IsACallInst().IsNil() && ptrUse.CalledValue() == callIntPtr {
203-
addFuncLoweringSwitch(mod, builder, funcID, ptrUse, func(funcPtr llvm.Value, params []llvm.Value) llvm.Value {
204-
return builder.CreateCall(funcPtr, params, "")
205-
}, functions)
206-
} else {
207-
panic("unexpected getFuncPtrCall")
208-
}
209-
ptrUse.EraseFromParentAsInstruction()
190+
} else {
191+
panic("unexpected getFuncPtrCall")
210192
}
211-
callIntPtr.EraseFromParentAsInstruction()
193+
ptrUse.EraseFromParentAsInstruction()
212194
}
213-
getFuncPtrCall.EraseFromParentAsInstruction()
195+
callIntPtr.EraseFromParentAsInstruction()
214196
}
197+
getFuncPtrCall.EraseFromParentAsInstruction()
215198
}
216199
}
217200
}
@@ -270,13 +253,18 @@ func addFuncLoweringSwitch(mod llvm.Module, builder llvm.Builder, funcID, call l
270253
phiBlocks[i] = bb
271254
phiValues[i] = result
272255
}
273-
// Create the PHI node so that the call result flows into the
274-
// next block (after the split). This is only necessary when the
275-
// call produced a value.
276256
if call.Type().TypeKind() != llvm.VoidTypeKind {
277-
builder.SetInsertPointBefore(nextBlock.FirstInstruction())
278-
phi := builder.CreatePHI(call.Type(), "")
279-
phi.AddIncoming(phiValues, phiBlocks)
280-
call.ReplaceAllUsesWith(phi)
257+
if len(functions) > 0 {
258+
// Create the PHI node so that the call result flows into the
259+
// next block (after the split). This is only necessary when the
260+
// call produced a value.
261+
builder.SetInsertPointBefore(nextBlock.FirstInstruction())
262+
phi := builder.CreatePHI(call.Type(), "")
263+
phi.AddIncoming(phiValues, phiBlocks)
264+
call.ReplaceAllUsesWith(phi)
265+
} else {
266+
// This is always a nil panic, so replace the call result with undef.
267+
call.ReplaceAllUsesWith(llvm.Undef(call.Type()))
268+
}
281269
}
282270
}

‎transform/testdata/func-lowering.ll‎

Lines changed: 10 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,9 @@ target triple = "wasm32-unknown-unknown-wasm"
44
%runtime.typecodeID = type { %runtime.typecodeID*, i32 }
55
%runtime.funcValueWithSignature = type { i32, %runtime.typecodeID* }
66

7-
@"reflect/types.type:func:{basic:int8}{}" = external constant %runtime.typecodeID
87
@"reflect/types.type:func:{basic:uint8}{}" = external constant %runtime.typecodeID
98
@"reflect/types.type:func:{basic:int}{}" = external constant %runtime.typecodeID
10-
@"funcInt8$withSignature" = constant %runtime.funcValueWithSignature { i32 ptrtoint (void (i8, i8*, i8*)* @funcInt8 to i32), %runtime.typecodeID* @"reflect/types.type:func:{basic:int8}{}" }
9+
@"reflect/types.type:func:{}{basic:uint32}" = external constant %runtime.typecodeID
1110
@"func1Uint8$withSignature" = constant %runtime.funcValueWithSignature { i32 ptrtoint (void (i8, i8*, i8*)* @func1Uint8 to i32), %runtime.typecodeID* @"reflect/types.type:func:{basic:uint8}{}" }
1211
@"func2Uint8$withSignature" = constant %runtime.funcValueWithSignature { i32 ptrtoint (void (i8, i8*, i8*)* @func2Uint8 to i32), %runtime.typecodeID* @"reflect/types.type:func:{basic:uint8}{}" }
1312
@"main$withSignature" = constant %runtime.funcValueWithSignature { i32 ptrtoint (void (i32, i8*, i8*)* @"main$1" to i32), %runtime.typecodeID* @"reflect/types.type:func:{basic:int}{}" }
@@ -23,29 +22,26 @@ declare void @"main$1"(i32, i8*, i8*)
2322

2423
declare void @"main$2"(i32, i8*, i8*)
2524

26-
declare void @funcInt8(i8, i8*, i8*)
27-
2825
declare void @func1Uint8(i8, i8*, i8*)
2926

3027
declare void @func2Uint8(i8, i8*, i8*)
3128

32-
; Call a function of which only one function with this signature is used as a
33-
; function value. This means that lowering it to IR is trivial: simply check
34-
; whether the func value is nil, and if not, call that one function directly.
35-
define void @runFunc1(i8*, i32, i8, i8* %context, i8* %parentHandle) {
29+
; There are no functions with this signature used in a func value.
30+
; This means that this should unconditionally nil panic.
31+
define i32 @runFuncNone(i8*, i32, i8* %context, i8* %parentHandle) {
3632
entry:
37-
%3 = call i32 @runtime.getFuncPtr(i8* %0, i32 %1, %runtime.typecodeID* @"reflect/types.type:func:{basic:int8}{}", i8* undef, i8* null)
38-
%4 = inttoptr i32 %3 to void (i8, i8*, i8*)*
39-
%5 = icmp eq void (i8, i8*, i8*)* %4, null
40-
br i1 %5, label %fpcall.nil, label %fpcall.next
33+
%2 = call i32 @runtime.getFuncPtr(i8* %0, i32 %1, %runtime.typecodeID* @"reflect/types.type:func:{}{basic:uint32}", i8* undef, i8* null)
34+
%3 = inttoptr i32 %2 to i32 (i8*, i8*)*
35+
%4 = icmp eq i32 (i8*, i8*)* %3, null
36+
br i1 %4, label %fpcall.nil, label %fpcall.next
4137

4238
fpcall.nil:
4339
call void @runtime.nilPanic(i8* undef, i8* null)
4440
unreachable
4541

4642
fpcall.next:
47-
call void %4(i8 %2, i8* %0, i8* undef)
48-
ret void
43+
%5 = call i32 %3(i8* %0, i8* undef)
44+
ret i32 %5
4945
}
5046

5147
; There are two functions with this signature used in a func value. That means

‎transform/testdata/func-lowering.out.ll‎

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,9 @@ target triple = "wasm32-unknown-unknown-wasm"
44
%runtime.typecodeID = type { %runtime.typecodeID*, i32 }
55
%runtime.funcValueWithSignature = type { i32, %runtime.typecodeID* }
66

7-
@"reflect/types.type:func:{basic:int8}{}" = external constant %runtime.typecodeID
87
@"reflect/types.type:func:{basic:uint8}{}" = external constant %runtime.typecodeID
98
@"reflect/types.type:func:{basic:int}{}" = external constant %runtime.typecodeID
10-
@"funcInt8$withSignature" = constant %runtime.funcValueWithSignature { i32 ptrtoint (void (i8, i8*, i8*)* @funcInt8 to i32), %runtime.typecodeID* @"reflect/types.type:func:{basic:int8}{}" }
9+
@"reflect/types.type:func:{}{basic:uint32}" = external constant %runtime.typecodeID
1110
@"func1Uint8$withSignature" = constant %runtime.funcValueWithSignature { i32 ptrtoint (void (i8, i8*, i8*)* @func1Uint8 to i32), %runtime.typecodeID* @"reflect/types.type:func:{basic:uint8}{}" }
1211
@"func2Uint8$withSignature" = constant %runtime.funcValueWithSignature { i32 ptrtoint (void (i8, i8*, i8*)* @func2Uint8 to i32), %runtime.typecodeID* @"reflect/types.type:func:{basic:uint8}{}" }
1312
@"main$withSignature" = constant %runtime.funcValueWithSignature { i32 ptrtoint (void (i32, i8*, i8*)* @"main$1" to i32), %runtime.typecodeID* @"reflect/types.type:func:{basic:int}{}" }
@@ -23,26 +22,32 @@ declare void @"main$1"(i32, i8*, i8*)
2322

2423
declare void @"main$2"(i32, i8*, i8*)
2524

26-
declare void @funcInt8(i8, i8*, i8*)
27-
2825
declare void @func1Uint8(i8, i8*, i8*)
2926

3027
declare void @func2Uint8(i8, i8*, i8*)
3128

32-
define void @runFunc1(i8* %0, i32 %1, i8 %2, i8* %context, i8* %parentHandle) {
29+
define i32 @runFuncNone(i8* %0, i32 %1, i8* %context, i8* %parentHandle) {
3330
entry:
34-
%3 = icmp eq i32 %1, 0
35-
%4 = select i1 %3, void (i8, i8*, i8*)* null, void (i8, i8*, i8*)* @funcInt8
36-
%5 = icmp eq void (i8, i8*, i8*)* %4, null
37-
br i1 %5, label %fpcall.nil, label %fpcall.next
31+
br i1 false, label %fpcall.nil, label %fpcall.next
3832

3933
fpcall.nil: ; preds = %entry
4034
call void @runtime.nilPanic(i8* undef, i8* null)
4135
unreachable
4236

4337
fpcall.next: ; preds = %entry
44-
call void %4(i8 %2, i8* %0, i8* undef)
45-
ret void
38+
switch i32 %1, label %func.default [
39+
i32 0, label %func.nil
40+
]
41+
42+
func.nil: ; preds = %fpcall.next
43+
call void @runtime.nilPanic(i8* undef, i8* null)
44+
unreachable
45+
46+
func.next: ; No predecessors!
47+
ret i32 undef
48+
49+
func.default: ; preds = %fpcall.next
50+
unreachable
4651
}
4752

4853
define void @runFunc2(i8* %0, i32 %1, i8 %2, i8* %context, i8* %parentHandle) {

0 commit comments

Comments
 (0)