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.
Этот коммит содержится в:
Jaden Weiss 2020-04-09 16:08:15 -04:00 коммит произвёл Ayke
родитель 0a8bfc57ef
коммит 9890c760cf
3 изменённых файлов: 110 добавлений и 121 удалений

Просмотреть файл

@ -6,6 +6,7 @@ package transform
import ( import (
"sort" "sort"
"strconv" "strconv"
"strings"
"github.com/tinygo-org/tinygo/compiler/llvmutil" "github.com/tinygo-org/tinygo/compiler/llvmutil"
"tinygo.org/x/go-llvm" "tinygo.org/x/go-llvm"
@ -55,17 +56,30 @@ func LowerFuncValues(mod llvm.Module) {
funcValueWithSignaturePtr := llvm.PointerType(mod.GetTypeByName("runtime.funcValueWithSignature"), 0) funcValueWithSignaturePtr := llvm.PointerType(mod.GetTypeByName("runtime.funcValueWithSignature"), 0)
signatures := map[string]*funcSignatureInfo{} signatures := map[string]*funcSignatureInfo{}
for global := mod.FirstGlobal(); !global.IsNil(); global = llvm.NextGlobal(global) { for global := mod.FirstGlobal(); !global.IsNil(); global = llvm.NextGlobal(global) {
if global.Type() != funcValueWithSignaturePtr { var sig, funcVal llvm.Value
switch {
case global.Type() == funcValueWithSignaturePtr:
sig = llvm.ConstExtractValue(global.Initializer(), []uint32{1})
funcVal = global
case strings.HasPrefix(global.Name(), "reflect/types.type:func:{"):
sig = global
default:
continue continue
} }
sig := llvm.ConstExtractValue(global.Initializer(), []uint32{1})
name := sig.Name() name := sig.Name()
var funcValueWithSignatures []llvm.Value
if funcVal.IsNil() {
funcValueWithSignatures = []llvm.Value{}
} else {
funcValueWithSignatures = []llvm.Value{funcVal}
}
if info, ok := signatures[name]; ok { if info, ok := signatures[name]; ok {
info.funcValueWithSignatures = append(info.funcValueWithSignatures, global) info.funcValueWithSignatures = append(info.funcValueWithSignatures, funcValueWithSignatures...)
} else { } else {
signatures[name] = &funcSignatureInfo{ signatures[name] = &funcSignatureInfo{
sig: sig, sig: sig,
funcValueWithSignatures: []llvm.Value{global}, funcValueWithSignatures: funcValueWithSignatures,
} }
} }
} }
@ -123,95 +137,64 @@ func LowerFuncValues(mod llvm.Module) {
panic("expected all call uses to be runtime.getFuncPtr") panic("expected all call uses to be runtime.getFuncPtr")
} }
funcID := getFuncPtrCall.Operand(1) funcID := getFuncPtrCall.Operand(1)
switch len(functions) {
case 0:
// There are no functions used in a func value that implement
// this signature. The only possible value is a nil value.
for _, inttoptr := range getUses(getFuncPtrCall) {
if inttoptr.IsAIntToPtrInst().IsNil() {
panic("expected inttoptr")
}
nilptr := llvm.ConstPointerNull(inttoptr.Type())
inttoptr.ReplaceAllUsesWith(nilptr)
inttoptr.EraseFromParentAsInstruction()
}
getFuncPtrCall.EraseFromParentAsInstruction()
case 1:
// There is exactly one function with this signature that is
// used in a func value. The func value itself can be either nil
// or this one function.
builder.SetInsertPointBefore(getFuncPtrCall)
zero := llvm.ConstInt(uintptrType, 0, false)
isnil := builder.CreateICmp(llvm.IntEQ, funcID, zero, "")
funcPtrNil := llvm.ConstPointerNull(functions[0].funcPtr.Type())
funcPtr := builder.CreateSelect(isnil, funcPtrNil, functions[0].funcPtr, "")
for _, inttoptr := range getUses(getFuncPtrCall) {
if inttoptr.IsAIntToPtrInst().IsNil() {
panic("expected inttoptr")
}
inttoptr.ReplaceAllUsesWith(funcPtr)
inttoptr.EraseFromParentAsInstruction()
}
getFuncPtrCall.EraseFromParentAsInstruction()
default:
// There are multiple functions used in a func value that
// implement this signature.
// What we'll do is transform the following:
// rawPtr := runtime.getFuncPtr(func.ptr)
// if rawPtr == nil {
// runtime.nilPanic()
// }
// result := rawPtr(...args, func.context)
// into this:
// if false {
// runtime.nilPanic()
// }
// var result // Phi
// switch fn.id {
// case 0:
// runtime.nilPanic()
// case 1:
// result = call first implementation...
// case 2:
// result = call second implementation...
// default:
// unreachable
// }
// Remove some casts, checks, and the old call which we're going // There are functions used in a func value that
// to replace. // implement this signature.
for _, callIntPtr := range getUses(getFuncPtrCall) { // What we'll do is transform the following:
if !callIntPtr.IsACallInst().IsNil() && callIntPtr.CalledValue().Name() == "internal/task.start" { // rawPtr := runtime.getFuncPtr(func.ptr)
// Special case for goroutine starts. // if rawPtr == nil {
addFuncLoweringSwitch(mod, builder, funcID, callIntPtr, func(funcPtr llvm.Value, params []llvm.Value) llvm.Value { // runtime.nilPanic()
i8ptrType := llvm.PointerType(ctx.Int8Type(), 0) // }
calleeValue := builder.CreatePtrToInt(funcPtr, uintptrType, "") // result := rawPtr(...args, func.context)
start := mod.NamedFunction("internal/task.start") // into this:
builder.CreateCall(start, []llvm.Value{calleeValue, callIntPtr.Operand(1), llvm.Undef(i8ptrType), llvm.ConstNull(i8ptrType)}, "") // if false {
return llvm.Value{} // void so no return value // runtime.nilPanic()
}, functions) // }
callIntPtr.EraseFromParentAsInstruction() // var result // Phi
continue // switch fn.id {
} // case 0:
if callIntPtr.IsAIntToPtrInst().IsNil() { // runtime.nilPanic()
panic("expected inttoptr") // case 1:
} // result = call first implementation...
for _, ptrUse := range getUses(callIntPtr) { // case 2:
if !ptrUse.IsAICmpInst().IsNil() { // result = call second implementation...
ptrUse.ReplaceAllUsesWith(llvm.ConstInt(ctx.Int1Type(), 0, false)) // default:
} else if !ptrUse.IsACallInst().IsNil() && ptrUse.CalledValue() == callIntPtr { // unreachable
addFuncLoweringSwitch(mod, builder, funcID, ptrUse, func(funcPtr llvm.Value, params []llvm.Value) llvm.Value { // }
return builder.CreateCall(funcPtr, params, "")
}, functions) // Remove some casts, checks, and the old call which we're going
} else { // to replace.
panic("unexpected getFuncPtrCall") for _, callIntPtr := range getUses(getFuncPtrCall) {
} if !callIntPtr.IsACallInst().IsNil() && callIntPtr.CalledValue().Name() == "internal/task.start" {
ptrUse.EraseFromParentAsInstruction() // Special case for goroutine starts.
} addFuncLoweringSwitch(mod, builder, funcID, callIntPtr, func(funcPtr llvm.Value, params []llvm.Value) llvm.Value {
i8ptrType := llvm.PointerType(ctx.Int8Type(), 0)
calleeValue := builder.CreatePtrToInt(funcPtr, uintptrType, "")
start := mod.NamedFunction("internal/task.start")
builder.CreateCall(start, []llvm.Value{calleeValue, callIntPtr.Operand(1), llvm.Undef(i8ptrType), llvm.ConstNull(i8ptrType)}, "")
return llvm.Value{} // void so no return value
}, functions)
callIntPtr.EraseFromParentAsInstruction() callIntPtr.EraseFromParentAsInstruction()
continue
} }
getFuncPtrCall.EraseFromParentAsInstruction() if callIntPtr.IsAIntToPtrInst().IsNil() {
panic("expected inttoptr")
}
for _, ptrUse := range getUses(callIntPtr) {
if !ptrUse.IsAICmpInst().IsNil() {
ptrUse.ReplaceAllUsesWith(llvm.ConstInt(ctx.Int1Type(), 0, false))
} else if !ptrUse.IsACallInst().IsNil() && ptrUse.CalledValue() == callIntPtr {
addFuncLoweringSwitch(mod, builder, funcID, ptrUse, func(funcPtr llvm.Value, params []llvm.Value) llvm.Value {
return builder.CreateCall(funcPtr, params, "")
}, functions)
} else {
panic("unexpected getFuncPtrCall")
}
ptrUse.EraseFromParentAsInstruction()
}
callIntPtr.EraseFromParentAsInstruction()
} }
getFuncPtrCall.EraseFromParentAsInstruction()
} }
} }
} }
@ -270,13 +253,18 @@ func addFuncLoweringSwitch(mod llvm.Module, builder llvm.Builder, funcID, call l
phiBlocks[i] = bb phiBlocks[i] = bb
phiValues[i] = result phiValues[i] = result
} }
// Create the PHI node so that the call result flows into the
// next block (after the split). This is only necessary when the
// call produced a value.
if call.Type().TypeKind() != llvm.VoidTypeKind { if call.Type().TypeKind() != llvm.VoidTypeKind {
builder.SetInsertPointBefore(nextBlock.FirstInstruction()) if len(functions) > 0 {
phi := builder.CreatePHI(call.Type(), "") // Create the PHI node so that the call result flows into the
phi.AddIncoming(phiValues, phiBlocks) // next block (after the split). This is only necessary when the
call.ReplaceAllUsesWith(phi) // call produced a value.
builder.SetInsertPointBefore(nextBlock.FirstInstruction())
phi := builder.CreatePHI(call.Type(), "")
phi.AddIncoming(phiValues, phiBlocks)
call.ReplaceAllUsesWith(phi)
} else {
// This is always a nil panic, so replace the call result with undef.
call.ReplaceAllUsesWith(llvm.Undef(call.Type()))
}
} }
} }

24
transform/testdata/func-lowering.ll предоставленный
Просмотреть файл

@ -4,10 +4,9 @@ target triple = "wasm32-unknown-unknown-wasm"
%runtime.typecodeID = type { %runtime.typecodeID*, i32 } %runtime.typecodeID = type { %runtime.typecodeID*, i32 }
%runtime.funcValueWithSignature = type { i32, %runtime.typecodeID* } %runtime.funcValueWithSignature = type { i32, %runtime.typecodeID* }
@"reflect/types.type:func:{basic:int8}{}" = external constant %runtime.typecodeID
@"reflect/types.type:func:{basic:uint8}{}" = external constant %runtime.typecodeID @"reflect/types.type:func:{basic:uint8}{}" = external constant %runtime.typecodeID
@"reflect/types.type:func:{basic:int}{}" = external constant %runtime.typecodeID @"reflect/types.type:func:{basic:int}{}" = external constant %runtime.typecodeID
@"funcInt8$withSignature" = constant %runtime.funcValueWithSignature { i32 ptrtoint (void (i8, i8*, i8*)* @funcInt8 to i32), %runtime.typecodeID* @"reflect/types.type:func:{basic:int8}{}" } @"reflect/types.type:func:{}{basic:uint32}" = external constant %runtime.typecodeID
@"func1Uint8$withSignature" = constant %runtime.funcValueWithSignature { i32 ptrtoint (void (i8, i8*, i8*)* @func1Uint8 to i32), %runtime.typecodeID* @"reflect/types.type:func:{basic:uint8}{}" } @"func1Uint8$withSignature" = constant %runtime.funcValueWithSignature { i32 ptrtoint (void (i8, i8*, i8*)* @func1Uint8 to i32), %runtime.typecodeID* @"reflect/types.type:func:{basic:uint8}{}" }
@"func2Uint8$withSignature" = constant %runtime.funcValueWithSignature { i32 ptrtoint (void (i8, i8*, i8*)* @func2Uint8 to i32), %runtime.typecodeID* @"reflect/types.type:func:{basic:uint8}{}" } @"func2Uint8$withSignature" = constant %runtime.funcValueWithSignature { i32 ptrtoint (void (i8, i8*, i8*)* @func2Uint8 to i32), %runtime.typecodeID* @"reflect/types.type:func:{basic:uint8}{}" }
@"main$withSignature" = constant %runtime.funcValueWithSignature { i32 ptrtoint (void (i32, i8*, i8*)* @"main$1" to i32), %runtime.typecodeID* @"reflect/types.type:func:{basic:int}{}" } @"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*)
declare void @"main$2"(i32, i8*, i8*) declare void @"main$2"(i32, i8*, i8*)
declare void @funcInt8(i8, i8*, i8*)
declare void @func1Uint8(i8, i8*, i8*) declare void @func1Uint8(i8, i8*, i8*)
declare void @func2Uint8(i8, i8*, i8*) declare void @func2Uint8(i8, i8*, i8*)
; Call a function of which only one function with this signature is used as a ; There are no functions with this signature used in a func value.
; function value. This means that lowering it to IR is trivial: simply check ; This means that this should unconditionally nil panic.
; whether the func value is nil, and if not, call that one function directly. define i32 @runFuncNone(i8*, i32, i8* %context, i8* %parentHandle) {
define void @runFunc1(i8*, i32, i8, i8* %context, i8* %parentHandle) {
entry: entry:
%3 = call i32 @runtime.getFuncPtr(i8* %0, i32 %1, %runtime.typecodeID* @"reflect/types.type:func:{basic:int8}{}", i8* undef, i8* null) %2 = call i32 @runtime.getFuncPtr(i8* %0, i32 %1, %runtime.typecodeID* @"reflect/types.type:func:{}{basic:uint32}", i8* undef, i8* null)
%4 = inttoptr i32 %3 to void (i8, i8*, i8*)* %3 = inttoptr i32 %2 to i32 (i8*, i8*)*
%5 = icmp eq void (i8, i8*, i8*)* %4, null %4 = icmp eq i32 (i8*, i8*)* %3, null
br i1 %5, label %fpcall.nil, label %fpcall.next br i1 %4, label %fpcall.nil, label %fpcall.next
fpcall.nil: fpcall.nil:
call void @runtime.nilPanic(i8* undef, i8* null) call void @runtime.nilPanic(i8* undef, i8* null)
unreachable unreachable
fpcall.next: fpcall.next:
call void %4(i8 %2, i8* %0, i8* undef) %5 = call i32 %3(i8* %0, i8* undef)
ret void ret i32 %5
} }
; There are two functions with this signature used in a func value. That means ; There are two functions with this signature used in a func value. That means

27
transform/testdata/func-lowering.out.ll предоставленный
Просмотреть файл

@ -4,10 +4,9 @@ target triple = "wasm32-unknown-unknown-wasm"
%runtime.typecodeID = type { %runtime.typecodeID*, i32 } %runtime.typecodeID = type { %runtime.typecodeID*, i32 }
%runtime.funcValueWithSignature = type { i32, %runtime.typecodeID* } %runtime.funcValueWithSignature = type { i32, %runtime.typecodeID* }
@"reflect/types.type:func:{basic:int8}{}" = external constant %runtime.typecodeID
@"reflect/types.type:func:{basic:uint8}{}" = external constant %runtime.typecodeID @"reflect/types.type:func:{basic:uint8}{}" = external constant %runtime.typecodeID
@"reflect/types.type:func:{basic:int}{}" = external constant %runtime.typecodeID @"reflect/types.type:func:{basic:int}{}" = external constant %runtime.typecodeID
@"funcInt8$withSignature" = constant %runtime.funcValueWithSignature { i32 ptrtoint (void (i8, i8*, i8*)* @funcInt8 to i32), %runtime.typecodeID* @"reflect/types.type:func:{basic:int8}{}" } @"reflect/types.type:func:{}{basic:uint32}" = external constant %runtime.typecodeID
@"func1Uint8$withSignature" = constant %runtime.funcValueWithSignature { i32 ptrtoint (void (i8, i8*, i8*)* @func1Uint8 to i32), %runtime.typecodeID* @"reflect/types.type:func:{basic:uint8}{}" } @"func1Uint8$withSignature" = constant %runtime.funcValueWithSignature { i32 ptrtoint (void (i8, i8*, i8*)* @func1Uint8 to i32), %runtime.typecodeID* @"reflect/types.type:func:{basic:uint8}{}" }
@"func2Uint8$withSignature" = constant %runtime.funcValueWithSignature { i32 ptrtoint (void (i8, i8*, i8*)* @func2Uint8 to i32), %runtime.typecodeID* @"reflect/types.type:func:{basic:uint8}{}" } @"func2Uint8$withSignature" = constant %runtime.funcValueWithSignature { i32 ptrtoint (void (i8, i8*, i8*)* @func2Uint8 to i32), %runtime.typecodeID* @"reflect/types.type:func:{basic:uint8}{}" }
@"main$withSignature" = constant %runtime.funcValueWithSignature { i32 ptrtoint (void (i32, i8*, i8*)* @"main$1" to i32), %runtime.typecodeID* @"reflect/types.type:func:{basic:int}{}" } @"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*)
declare void @"main$2"(i32, i8*, i8*) declare void @"main$2"(i32, i8*, i8*)
declare void @funcInt8(i8, i8*, i8*)
declare void @func1Uint8(i8, i8*, i8*) declare void @func1Uint8(i8, i8*, i8*)
declare void @func2Uint8(i8, i8*, i8*) declare void @func2Uint8(i8, i8*, i8*)
define void @runFunc1(i8* %0, i32 %1, i8 %2, i8* %context, i8* %parentHandle) { define i32 @runFuncNone(i8* %0, i32 %1, i8* %context, i8* %parentHandle) {
entry: entry:
%3 = icmp eq i32 %1, 0 br i1 false, label %fpcall.nil, label %fpcall.next
%4 = select i1 %3, void (i8, i8*, i8*)* null, void (i8, i8*, i8*)* @funcInt8
%5 = icmp eq void (i8, i8*, i8*)* %4, null
br i1 %5, label %fpcall.nil, label %fpcall.next
fpcall.nil: ; preds = %entry fpcall.nil: ; preds = %entry
call void @runtime.nilPanic(i8* undef, i8* null) call void @runtime.nilPanic(i8* undef, i8* null)
unreachable unreachable
fpcall.next: ; preds = %entry fpcall.next: ; preds = %entry
call void %4(i8 %2, i8* %0, i8* undef) switch i32 %1, label %func.default [
ret void i32 0, label %func.nil
]
func.nil: ; preds = %fpcall.next
call void @runtime.nilPanic(i8* undef, i8* null)
unreachable
func.next: ; No predecessors!
ret i32 undef
func.default: ; preds = %fpcall.next
unreachable
} }
define void @runFunc2(i8* %0, i32 %1, i8 %2, i8* %context, i8* %parentHandle) { define void @runFunc2(i8* %0, i32 %1, i8 %2, i8* %context, i8* %parentHandle) {