From 6e2e2a0a741ecc2b96357b2198218f4c4d391d79 Mon Sep 17 00:00:00 2001 From: Aaron Turner Date: Thu, 28 Jan 2021 12:53:27 -0800 Subject: [PATCH 01/13] Started logging out some stuff to figure out what's going on in external functions --- compiler/compiler.go | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/compiler/compiler.go b/compiler/compiler.go index a7887f3669..5c55abc628 100644 --- a/compiler/compiler.go +++ b/compiler/compiler.go @@ -283,9 +283,20 @@ func CompileProgram(lprogram *loader.Program, machine llvm.TargetMachine, config initFuncs = append(initFuncs, c.getFunction(f)) } if f.Blocks == nil { + fmt.Printf("External Function: %v \n\n%#v\n\n%#v\n\n", f, f, f.targets); continue // external function } + // TODO torch2424: Trying to fix bug #1559, to allow for non-conflicting wasm import names + // Actually, it's a bit more complicated + // The name really needs to be write on non-wasm systems but apparently the symbol should not be renamed on wasm + // The IR is currently generated here: + // https://github.com/tinygo-org/tinygo/blob/release/compiler/compiler.go#L703 + // But things will change with this PR: + // https://github.com/tinygo-org/tinygo/pull/1584 + // fmt.Printf("Yooooo \n\n%+v\n\n", config.Options); + // fmt.Printf("WasmAbi \n\n%s\n\n", config.Options.WasmAbi); + // Create the function definition. b := newBuilder(c, irbuilder, f) b.createFunction() From cd0b7ad64307312a4d0da6ef6de90557b8cd6d79 Mon Sep 17 00:00:00 2001 From: Aaron Turner Date: Fri, 19 Feb 2021 18:00:58 -0800 Subject: [PATCH 02/13] Started taking a look at the Symbol.go code for the wasi fix --- compiler/compiler.go | 2 +- compiler/symbol.go | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/compiler/compiler.go b/compiler/compiler.go index 5c55abc628..b02a3fb59b 100644 --- a/compiler/compiler.go +++ b/compiler/compiler.go @@ -283,7 +283,7 @@ func CompileProgram(lprogram *loader.Program, machine llvm.TargetMachine, config initFuncs = append(initFuncs, c.getFunction(f)) } if f.Blocks == nil { - fmt.Printf("External Function: %v \n\n%#v\n\n%#v\n\n", f, f, f.targets); + // fmt.Printf("External Function: %v \n\n%#v\n\n%#v\n\n", f, f, f.targets); continue // external function } diff --git a/compiler/symbol.go b/compiler/symbol.go index 348eb52551..3da98d6ba4 100644 --- a/compiler/symbol.go +++ b/compiler/symbol.go @@ -10,6 +10,8 @@ import ( "strconv" "strings" + "fmt" + "github.com/tinygo-org/tinygo/loader" "golang.org/x/tools/go/ssa" "tinygo.org/x/go-llvm" @@ -166,6 +168,14 @@ func (c *compilerContext) getFunction(fn *ssa.Function) llvm.Value { return llvmFn } + +// TODO (torch2424): +// Ayke said: +// Actually, functions are never renamed +// The name you're looking for is stored in functionInfo.linkName, which is determined here: +// `getFunctionInfo` +// Right below it is parsePragmas, which is where the //go: pragmas are parsed. + // getFunctionInfo returns information about a function that is not directly // present in *ssa.Function, such as the link name and whether it should be // exported. From 5b03fdab59e483fdbe9e36561e89eaef907242b1 Mon Sep 17 00:00:00 2001 From: Aaron Turner Date: Tue, 23 Feb 2021 18:10:13 -0800 Subject: [PATCH 03/13] Got the function name to correctly change in the symbols --- compiler/symbol.go | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/compiler/symbol.go b/compiler/symbol.go index 3da98d6ba4..194062c699 100644 --- a/compiler/symbol.go +++ b/compiler/symbol.go @@ -216,8 +216,18 @@ func (info *functionInfo) parsePragmas(f *ssa.Function) { case "//go:export": if len(parts) != 2 { continue - } + } + info.linkName = parts[1] + + if info.linkName == "write" { + // TODO: torch2424 + info.linkName = "yo"; + } + + // TODO: torch2424 + fmt.Printf("torch2424 go:export \n\n%+v\n\n", info); + info.exported = true case "//go:wasm-module": // Alternative comment for setting the import module. From 8f89d4d540f73194449494d87a711adebaef47b9 Mon Sep 17 00:00:00 2001 From: Aaron Turner Date: Wed, 24 Feb 2021 18:05:48 -0800 Subject: [PATCH 04/13] Namespaced all of the wasm import functions --- compiler/compiler.go | 10 ---------- compiler/symbol.go | 21 +++++++++++++-------- 2 files changed, 13 insertions(+), 18 deletions(-) diff --git a/compiler/compiler.go b/compiler/compiler.go index b02a3fb59b..f555c2a21a 100644 --- a/compiler/compiler.go +++ b/compiler/compiler.go @@ -287,16 +287,6 @@ func CompileProgram(lprogram *loader.Program, machine llvm.TargetMachine, config continue // external function } - // TODO torch2424: Trying to fix bug #1559, to allow for non-conflicting wasm import names - // Actually, it's a bit more complicated - // The name really needs to be write on non-wasm systems but apparently the symbol should not be renamed on wasm - // The IR is currently generated here: - // https://github.com/tinygo-org/tinygo/blob/release/compiler/compiler.go#L703 - // But things will change with this PR: - // https://github.com/tinygo-org/tinygo/pull/1584 - // fmt.Printf("Yooooo \n\n%+v\n\n", config.Options); - // fmt.Printf("WasmAbi \n\n%s\n\n", config.Options.WasmAbi); - // Create the function definition. b := newBuilder(c, irbuilder, f) b.createFunction() diff --git a/compiler/symbol.go b/compiler/symbol.go index 194062c699..71b72918b0 100644 --- a/compiler/symbol.go +++ b/compiler/symbol.go @@ -24,7 +24,8 @@ import ( // present. type functionInfo struct { module string // go:wasm-module - linkName string // go:linkname, go:export + importName string // go:linkname, go:export - The name the developer assigns + linkName string // go:linkname, go:export - The name that we map for the particular module -> importName exported bool // go:export, CGo nobounds bool // go:nobounds variadic bool // go:variadic (CGo only) @@ -153,8 +154,13 @@ func (c *compilerContext) getFunction(fn *ssa.Function) llvm.Value { if info.exported { // Set the wasm-import-module attribute if the function's module is set. if info.module != "" { + + // We need to add the wasm-import-module and the wasm-import-name wasmImportModuleAttr := c.ctx.CreateStringAttribute("wasm-import-module", info.module) llvmFn.AddFunctionAttr(wasmImportModuleAttr) + + wasmImportNameAttr := c.ctx.CreateStringAttribute("wasm-import-name", info.importName) + llvmFn.AddFunctionAttr(wasmImportNameAttr) } nocaptureKind := llvm.AttributeKindID("nocapture") nocapture := c.ctx.CreateEnumAttribute(nocaptureKind, 0) @@ -218,16 +224,15 @@ func (info *functionInfo) parsePragmas(f *ssa.Function) { continue } - info.linkName = parts[1] + // Set the wasmimport name and the llvm link name + info.importName = parts[1]; - if info.linkName == "write" { - // TODO: torch2424 - info.linkName = "yo"; + if info.importName == "_start" { + info.linkName = info.importName; + } else { + info.linkName = fmt.Sprintf("wasm_import_%s_%s", info.module, info.importName); } - // TODO: torch2424 - fmt.Printf("torch2424 go:export \n\n%+v\n\n", info); - info.exported = true case "//go:wasm-module": // Alternative comment for setting the import module. From 3d9b7083529e29a412fc46c324fd3f08218f6d90 Mon Sep 17 00:00:00 2001 From: Aaron Turner Date: Wed, 24 Feb 2021 18:12:17 -0800 Subject: [PATCH 05/13] Fixed issues with no module imports --- compiler/symbol.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/symbol.go b/compiler/symbol.go index 71b72918b0..22af9b6ce7 100644 --- a/compiler/symbol.go +++ b/compiler/symbol.go @@ -227,10 +227,10 @@ func (info *functionInfo) parsePragmas(f *ssa.Function) { // Set the wasmimport name and the llvm link name info.importName = parts[1]; - if info.importName == "_start" { + if info.module == "" { info.linkName = info.importName; } else { - info.linkName = fmt.Sprintf("wasm_import_%s_%s", info.module, info.importName); + info.linkName = fmt.Sprintf("tinygo_wasm_import_%s_%s", info.module, info.importName); } info.exported = true From 37626e6c59dca04251f7c467aa9715e9bc3a3f6c Mon Sep 17 00:00:00 2001 From: Aaron Turner Date: Wed, 24 Feb 2021 19:41:57 -0800 Subject: [PATCH 06/13] Fixed the PR so it's actually good to go --- compiler/compiler.go | 1 - compiler/symbol.go | 8 -------- 2 files changed, 9 deletions(-) diff --git a/compiler/compiler.go b/compiler/compiler.go index f555c2a21a..a7887f3669 100644 --- a/compiler/compiler.go +++ b/compiler/compiler.go @@ -283,7 +283,6 @@ func CompileProgram(lprogram *loader.Program, machine llvm.TargetMachine, config initFuncs = append(initFuncs, c.getFunction(f)) } if f.Blocks == nil { - // fmt.Printf("External Function: %v \n\n%#v\n\n%#v\n\n", f, f, f.targets); continue // external function } diff --git a/compiler/symbol.go b/compiler/symbol.go index 22af9b6ce7..ea78eb3c7b 100644 --- a/compiler/symbol.go +++ b/compiler/symbol.go @@ -174,14 +174,6 @@ func (c *compilerContext) getFunction(fn *ssa.Function) llvm.Value { return llvmFn } - -// TODO (torch2424): -// Ayke said: -// Actually, functions are never renamed -// The name you're looking for is stored in functionInfo.linkName, which is determined here: -// `getFunctionInfo` -// Right below it is parsePragmas, which is where the //go: pragmas are parsed. - // getFunctionInfo returns information about a function that is not directly // present in *ssa.Function, such as the link name and whether it should be // exported. From 226b52e2e3b79fba2c49ae5725fb85dc6d32e9c8 Mon Sep 17 00:00:00 2001 From: Aaron Turner Date: Thu, 25 Feb 2021 10:37:37 -0800 Subject: [PATCH 07/13] Ran go fmt, and removed a newline --- compiler/symbol.go | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/compiler/symbol.go b/compiler/symbol.go index ea78eb3c7b..a21d4cadcc 100644 --- a/compiler/symbol.go +++ b/compiler/symbol.go @@ -4,14 +4,13 @@ package compiler // pragmas, determines the link name, etc. import ( + "fmt" "go/ast" "go/token" "go/types" "strconv" "strings" - "fmt" - "github.com/tinygo-org/tinygo/loader" "golang.org/x/tools/go/ssa" "tinygo.org/x/go-llvm" @@ -155,12 +154,12 @@ func (c *compilerContext) getFunction(fn *ssa.Function) llvm.Value { // Set the wasm-import-module attribute if the function's module is set. if info.module != "" { - // We need to add the wasm-import-module and the wasm-import-name + // We need to add the wasm-import-module and the wasm-import-name wasmImportModuleAttr := c.ctx.CreateStringAttribute("wasm-import-module", info.module) llvmFn.AddFunctionAttr(wasmImportModuleAttr) - wasmImportNameAttr := c.ctx.CreateStringAttribute("wasm-import-name", info.importName) - llvmFn.AddFunctionAttr(wasmImportNameAttr) + wasmImportNameAttr := c.ctx.CreateStringAttribute("wasm-import-name", info.importName) + llvmFn.AddFunctionAttr(wasmImportNameAttr) } nocaptureKind := llvm.AttributeKindID("nocapture") nocapture := c.ctx.CreateEnumAttribute(nocaptureKind, 0) @@ -214,16 +213,16 @@ func (info *functionInfo) parsePragmas(f *ssa.Function) { case "//go:export": if len(parts) != 2 { continue - } + } - // Set the wasmimport name and the llvm link name - info.importName = parts[1]; + // Set the wasmimport name and the llvm link name + info.importName = parts[1] - if info.module == "" { - info.linkName = info.importName; - } else { - info.linkName = fmt.Sprintf("tinygo_wasm_import_%s_%s", info.module, info.importName); - } + if info.module == "" { + info.linkName = info.importName + } else { + info.linkName = fmt.Sprintf("tinygo_wasm_import_%s_%s", info.module, info.importName) + } info.exported = true case "//go:wasm-module": From 6baf53a5ee21c253d17463b4557e987f8fe8f6f7 Mon Sep 17 00:00:00 2001 From: Aaron Turner Date: Thu, 25 Feb 2021 11:17:38 -0800 Subject: [PATCH 08/13] Ran the fmt from the makefile --- compiler/symbol.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/compiler/symbol.go b/compiler/symbol.go index a21d4cadcc..55bf2b4d6d 100644 --- a/compiler/symbol.go +++ b/compiler/symbol.go @@ -22,13 +22,13 @@ import ( // The linkName value contains a valid link name, even if //go:linkname is not // present. type functionInfo struct { - module string // go:wasm-module - importName string // go:linkname, go:export - The name the developer assigns - linkName string // go:linkname, go:export - The name that we map for the particular module -> importName - exported bool // go:export, CGo - nobounds bool // go:nobounds - variadic bool // go:variadic (CGo only) - inline inlineType // go:inline + module string // go:wasm-module + importName string // go:linkname, go:export - The name the developer assigns + linkName string // go:linkname, go:export - The name that we map for the particular module -> importName + exported bool // go:export, CGo + nobounds bool // go:nobounds + variadic bool // go:variadic (CGo only) + inline inlineType // go:inline } type inlineType int From 5d1e43a5e2c589dd920376ab9c5946b483174a76 Mon Sep 17 00:00:00 2001 From: Aaron Turner Date: Thu, 25 Feb 2021 19:15:36 -0800 Subject: [PATCH 09/13] Started making the suggested fixes, but got a little stuck --- compiler/symbol.go | 44 ++++++++++++++++++++++++++++++++++---------- 1 file changed, 34 insertions(+), 10 deletions(-) diff --git a/compiler/symbol.go b/compiler/symbol.go index 55bf2b4d6d..5e5f68c682 100644 --- a/compiler/symbol.go +++ b/compiler/symbol.go @@ -4,7 +4,7 @@ package compiler // pragmas, determines the link name, etc. import ( - "fmt" + "fmt" "go/ast" "go/token" "go/types" @@ -158,8 +158,11 @@ func (c *compilerContext) getFunction(fn *ssa.Function) llvm.Value { wasmImportModuleAttr := c.ctx.CreateStringAttribute("wasm-import-module", info.module) llvmFn.AddFunctionAttr(wasmImportModuleAttr) - wasmImportNameAttr := c.ctx.CreateStringAttribute("wasm-import-name", info.importName) - llvmFn.AddFunctionAttr(wasmImportNameAttr) + // Add the Wasm Import Name, if we are a named wasm import + if info.importName != "" { + wasmImportNameAttr := c.ctx.CreateStringAttribute("wasm-import-name", info.importName) + llvmFn.AddFunctionAttr(wasmImportNameAttr) + } } nocaptureKind := llvm.AttributeKindID("nocapture") nocapture := c.ctx.CreateEnumAttribute(nocaptureKind, 0) @@ -188,6 +191,7 @@ func (c *compilerContext) getFunctionInfo(f *ssa.Function) functionInfo { } // Check for //go: pragmas, which may change the link name (among others). info.parsePragmas(f) + fmt.Println(info); return info } @@ -198,6 +202,10 @@ func (info *functionInfo) parsePragmas(f *ssa.Function) { return } if decl, ok := f.Syntax().(*ast.FuncDecl); ok && decl.Doc != nil { + + // Our importName for a wasm module (if we are compiling to wasm), or llvm link name + var importName string = "" + for _, comment := range decl.Doc.List { text := comment.Text if strings.HasPrefix(text, "//export ") { @@ -215,14 +223,10 @@ func (info *functionInfo) parsePragmas(f *ssa.Function) { continue } - // Set the wasmimport name and the llvm link name - info.importName = parts[1] + importName = parts[1] + + fmt.Println("Uiiiii", importName); - if info.module == "" { - info.linkName = info.importName - } else { - info.linkName = fmt.Sprintf("tinygo_wasm_import_%s_%s", info.module, info.importName) - } info.exported = true case "//go:wasm-module": @@ -265,8 +269,28 @@ func (info *functionInfo) parsePragmas(f *ssa.Function) { info.variadic = true } } + + fmt.Println("Yooo", info.module, info.linkName, info.importName); + + + // TODO: torch2424 Why is this not working?!?! unexpected signal during runtime execution + // Set the importName for our exported function + if info.module == "" { + info.linkName = importName + } else { + // WebAssembly import + info.importName = importName + } + + } + + fmt.Println("TTTTggghhhh"); + + } + + } // globalInfo contains some information about a specific global. By default, From 19eb66f43f3f955a76de3325a865d02f597b7e46 Mon Sep 17 00:00:00 2001 From: Aaron Turner Date: Fri, 26 Feb 2021 12:49:31 -0800 Subject: [PATCH 10/13] Made requested changes --- compiler/symbol.go | 24 +++++------------------- 1 file changed, 5 insertions(+), 19 deletions(-) diff --git a/compiler/symbol.go b/compiler/symbol.go index 5e5f68c682..b87b01d378 100644 --- a/compiler/symbol.go +++ b/compiler/symbol.go @@ -4,7 +4,6 @@ package compiler // pragmas, determines the link name, etc. import ( - "fmt" "go/ast" "go/token" "go/types" @@ -191,7 +190,6 @@ func (c *compilerContext) getFunctionInfo(f *ssa.Function) functionInfo { } // Check for //go: pragmas, which may change the link name (among others). info.parsePragmas(f) - fmt.Println(info); return info } @@ -224,10 +222,6 @@ func (info *functionInfo) parsePragmas(f *ssa.Function) { } importName = parts[1] - - fmt.Println("Uiiiii", importName); - - info.exported = true case "//go:wasm-module": // Alternative comment for setting the import module. @@ -269,28 +263,20 @@ func (info *functionInfo) parsePragmas(f *ssa.Function) { info.variadic = true } } + } - fmt.Println("Yooo", info.module, info.linkName, info.importName); - - - // TODO: torch2424 Why is this not working?!?! unexpected signal during runtime execution - // Set the importName for our exported function + // Set the importName for our exported function if we have one + if importName != "" { if info.module == "" { info.linkName = importName } else { // WebAssembly import + // info.linkName = fmt.Sprintf("tinygo_wasm_import_%s_%s", info.module, info.importName) info.importName = importName } - - - } - - fmt.Println("TTTTggghhhh"); - + } } - - } // globalInfo contains some information about a specific global. By default, From 5bf39600e1b4491c41ea7c2281a5a1d085c24c98 Mon Sep 17 00:00:00 2001 From: Aaron Turner Date: Fri, 26 Feb 2021 12:50:27 -0800 Subject: [PATCH 11/13] Ran the formatter --- compiler/symbol.go | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/compiler/symbol.go b/compiler/symbol.go index b87b01d378..34c388934c 100644 --- a/compiler/symbol.go +++ b/compiler/symbol.go @@ -157,11 +157,11 @@ func (c *compilerContext) getFunction(fn *ssa.Function) llvm.Value { wasmImportModuleAttr := c.ctx.CreateStringAttribute("wasm-import-module", info.module) llvmFn.AddFunctionAttr(wasmImportModuleAttr) - // Add the Wasm Import Name, if we are a named wasm import - if info.importName != "" { - wasmImportNameAttr := c.ctx.CreateStringAttribute("wasm-import-name", info.importName) - llvmFn.AddFunctionAttr(wasmImportNameAttr) - } + // Add the Wasm Import Name, if we are a named wasm import + if info.importName != "" { + wasmImportNameAttr := c.ctx.CreateStringAttribute("wasm-import-name", info.importName) + llvmFn.AddFunctionAttr(wasmImportNameAttr) + } } nocaptureKind := llvm.AttributeKindID("nocapture") nocapture := c.ctx.CreateEnumAttribute(nocaptureKind, 0) @@ -201,8 +201,8 @@ func (info *functionInfo) parsePragmas(f *ssa.Function) { } if decl, ok := f.Syntax().(*ast.FuncDecl); ok && decl.Doc != nil { - // Our importName for a wasm module (if we are compiling to wasm), or llvm link name - var importName string = "" + // Our importName for a wasm module (if we are compiling to wasm), or llvm link name + var importName string = "" for _, comment := range decl.Doc.List { text := comment.Text @@ -221,7 +221,7 @@ func (info *functionInfo) parsePragmas(f *ssa.Function) { continue } - importName = parts[1] + importName = parts[1] info.exported = true case "//go:wasm-module": // Alternative comment for setting the import module. @@ -265,16 +265,16 @@ func (info *functionInfo) parsePragmas(f *ssa.Function) { } } - // Set the importName for our exported function if we have one - if importName != "" { - if info.module == "" { - info.linkName = importName - } else { - // WebAssembly import - // info.linkName = fmt.Sprintf("tinygo_wasm_import_%s_%s", info.module, info.importName) - info.importName = importName - } - } + // Set the importName for our exported function if we have one + if importName != "" { + if info.module == "" { + info.linkName = importName + } else { + // WebAssembly import + // info.linkName = fmt.Sprintf("tinygo_wasm_import_%s_%s", info.module, info.importName) + info.importName = importName + } + } } } From 20de259e8bb14c8d3ae1786079de70b2026309dc Mon Sep 17 00:00:00 2001 From: Aaron Turner Date: Fri, 26 Feb 2021 13:02:56 -0800 Subject: [PATCH 12/13] Removed a bad comment --- compiler/symbol.go | 1 - 1 file changed, 1 deletion(-) diff --git a/compiler/symbol.go b/compiler/symbol.go index 34c388934c..05626c16f9 100644 --- a/compiler/symbol.go +++ b/compiler/symbol.go @@ -271,7 +271,6 @@ func (info *functionInfo) parsePragmas(f *ssa.Function) { info.linkName = importName } else { // WebAssembly import - // info.linkName = fmt.Sprintf("tinygo_wasm_import_%s_%s", info.module, info.importName) info.importName = importName } } From 11a752052957a0a3c7146d1221a987dae67ed313 Mon Sep 17 00:00:00 2001 From: Aaron Turner Date: Tue, 2 Mar 2021 17:43:32 -0800 Subject: [PATCH 13/13] Made requested changes --- compiler/symbol.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/symbol.go b/compiler/symbol.go index 05626c16f9..b819b5abdd 100644 --- a/compiler/symbol.go +++ b/compiler/symbol.go @@ -202,7 +202,7 @@ func (info *functionInfo) parsePragmas(f *ssa.Function) { if decl, ok := f.Syntax().(*ast.FuncDecl); ok && decl.Doc != nil { // Our importName for a wasm module (if we are compiling to wasm), or llvm link name - var importName string = "" + var importName string for _, comment := range decl.Doc.List { text := comment.Text