compiler: pass large aggregate parameters indirectly (#5615)

compiler/calls.go

@@ -15,6 +15,15 @@ import (
 // a struct contains more fields, it is passed as a struct without expanding.
 const maxFieldsPerParam = 3
 
+// The maximum number of scalar leaves an aggregate parameter may have before
+// it is passed by pointer to a caller-owned copy instead of by value. Some
+// backends (notably WebAssembly) flatten aggregate parameters into individual
+// scalar parameters, and the WebAssembly JS embedding rejects function types
+// with more than 1000 parameters. See paramNeedsSpill.
+// With at most 16 scalars per parameter, a function needs over 60 such
+// parameters to approach the 1000-parameter limit.
+const maxLeavesPerParam = 16
+
 // paramInfo contains some information collected about a function parameter,
 // useful while declaring or defining a function.
 type paramInfo struct {
@@ -105,7 +114,7 @@ func (b *builder) createInvoke(fnType llvm.Type, fn llvm.Value, args []llvm.Valu
 // Expand an argument type to a list that can be used in a function call
 // parameter list.
 func (c *compilerContext) expandFormalParamType(t llvm.Type, name string, goType types.Type) []paramInfo {
-	if c.isIndirectAggregate(t) {
+	if c.paramNeedsSpill(t) {
 		return []paramInfo{{
 			llvmType: c.dataPtrType,
 			name:     name,
@@ -138,7 +147,7 @@ func (c *compilerContext) storedParamType(t llvm.Type, exported bool) llvm.Type
 }
 
 func (c *compilerContext) isIndirectParam(t llvm.Type, exported bool) bool {
-	return !exported && c.isIndirectAggregate(t)
+	return !exported && c.paramNeedsSpill(t)
 }
 
 func (b *builder) appendStoredValueTypes(valueTypes []llvm.Type, values []ssa.Value, exported bool) []llvm.Type {
@@ -298,6 +307,51 @@ func extractSubfield(t types.Type, field int) types.Type {
 	}
 }
 
+// countFlattenedLeaves returns the number of scalar values this type would be
+// flattened into when passed by value. Unlike flattenAggregateType, it looks
+// through arrays as well: LLVM's SelectionDAG (ComputeValueVTs) scalarizes
+// both structs and arrays in by-value aggregate parameters.
+func (c *compilerContext) countFlattenedLeaves(t llvm.Type) int {
+	switch t.TypeKind() {
+	case llvm.StructTypeKind:
+		count := 0
+		for _, field := range t.StructElementTypes() {
+			if c.targetData.TypeAllocSize(field) == 0 {
+				continue
+			}
+			count += c.countFlattenedLeaves(field)
+			if count >= 1<<30 {
+				// Saturate instead of overflowing, like the array case below.
+				return 1 << 30
+			}
+		}
+		return count
+	case llvm.ArrayTypeKind:
+		elemLeaves := c.countFlattenedLeaves(t.ElementType())
+		length := t.ArrayLength()
+		if elemLeaves > 0 && length > (1<<30)/elemLeaves {
+			// Saturate instead of overflowing; this is already far above
+			// any spill threshold.
+			return 1 << 30
+		}
+		return length * elemLeaves
+	default:
+		return 1
+	}
+}
+
+// paramNeedsSpill returns whether a parameter of this type must be passed by
+// pointer to a caller-owned copy instead of by value in the Go-internal
+// calling convention, to avoid creating functions with enormous numbers of
+// parameters once the backend flattens the aggregate.
+func (c *compilerContext) paramNeedsSpill(t llvm.Type) bool {
+	switch t.TypeKind() {
+	case llvm.StructTypeKind, llvm.ArrayTypeKind:
+		return c.countFlattenedLeaves(t) > maxLeavesPerParam
+	}
+	return false
+}
+
 // flattenAggregateTypeOffsets returns the offsets from the start of an object of
 // type t if this object were flattened like in flattenAggregate. Used together
 // with flattenAggregate to know the start indices of each value in the

compiler/calls_test.go

@@ -0,0 +1,53 @@
+package compiler
+
+import (
+	"testing"
+
+	"tinygo.org/x/go-llvm"
+)
+
+func TestParamNeedsSpill(t *testing.T) {
+	t.Parallel()
+	ctx := llvm.NewContext()
+	defer ctx.Dispose()
+	targetData := llvm.NewTargetData("e-m:e-p:32:32-p10:8:8-p20:8:8-i64:64-i128:128-n32:64-S128-ni:1:10:20")
+	defer targetData.Dispose()
+	c := &compilerContext{ctx: ctx, targetData: targetData}
+
+	i32 := ctx.Int32Type()
+	makeStruct := func(n int) llvm.Type {
+		fields := make([]llvm.Type, n)
+		for i := range fields {
+			fields[i] = i32
+		}
+		return ctx.StructType(fields, false)
+	}
+
+	for _, tc := range []struct {
+		name   string
+		typ    llvm.Type
+		leaves int
+		spill  bool
+	}{
+		{"i32", i32, 1, false},
+		{"empty", makeStruct(0), 0, false},
+		{"flat16", makeStruct(16), 16, false},
+		{"flat17", makeStruct(17), 17, true},
+		{"nested16", ctx.StructType([]llvm.Type{makeStruct(8), makeStruct(8)}, false), 16, false},
+		{"nested17", ctx.StructType([]llvm.Type{makeStruct(8), makeStruct(9)}, false), 17, true},
+		{"zeroSizeField", ctx.StructType([]llvm.Type{ctx.StructType(nil, false), i32}, false), 1, false},
+		{"array16", llvm.ArrayType(i32, 16), 16, false},
+		{"array17", llvm.ArrayType(i32, 17), 17, true},
+		{"structWithArray", ctx.StructType([]llvm.Type{llvm.ArrayType(i32, 16), i32}, false), 17, true},
+		{"hugeArray", llvm.ArrayType(makeStruct(4), 1<<29), 1 << 30, true},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			if leaves := c.countFlattenedLeaves(tc.typ); leaves != tc.leaves {
+				t.Errorf("countFlattenedLeaves = %d, want %d", leaves, tc.leaves)
+			}
+			if spill := c.paramNeedsSpill(tc.typ); spill != tc.spill {
+				t.Errorf("paramNeedsSpill = %v, want %v", spill, tc.spill)
+			}
+		})
+	}
+}

compiler/compiler.go

@@ -1444,8 +1444,13 @@ func (b *builder) createFunction() {
 	for _, phi := range b.phis {
 		block := phi.ssa.Block()
 		for i, edge := range phi.ssa.Edges {
-			llvmVal := b.getCallArgument(edge, false)
 			llvmBlock := b.blockInfo[block.Preds[i].Index].exit
+			// Materializing an indirect phi input may emit an allocation and a
+			// copy. Put those instructions in the corresponding predecessor,
+			// before its terminator, rather than at the builder's current end
+			// position after the function body has been emitted.
+			b.SetInsertPointBefore(llvmBlock.LastInstruction())
+			llvmVal := b.getCallArgument(edge, false)
 			phi.llvm.AddIncoming([]llvm.Value{llvmVal}, []llvm.BasicBlock{llvmBlock})
 		}
 	}

compiler/compiler_test.go

@@ -10,6 +10,7 @@ import (
 	"testing"
 
 	"github.com/tinygo-org/tinygo/compileopts"
+	"github.com/tinygo-org/tinygo/compiler/llvmutil"
 	"github.com/tinygo-org/tinygo/goenv"
 	"github.com/tinygo-org/tinygo/loader"
 	"tinygo.org/x/go-llvm"
@@ -53,6 +54,7 @@ func TestCompiler(t *testing.T) {
 		{"zeromap.go", "", ""},
 		{"generics.go", "", ""},
 		{"large.go", "", ""},
+		{"paramspill.go", "", ""},
 	}
 	if goMinor >= 20 {
 		tests = append(tests, testCase{"go1.20.go", "", ""})
@@ -97,9 +99,18 @@ func TestCompiler(t *testing.T) {
 			}
 
 			// Optimize IR a little.
+			// Run instcombine without fixpoint verification: standalone textual
+			// instcombine fatally aborts when it needs more than one iteration
+			// (an LLVM 18+ testing aid); real pass pipelines run it with
+			// no-verify-fixpoint (see transform/optimizer.go).
+			passes := "instcombine<no-verify-fixpoint>"
+			if llvmutil.Version() < 18 {
+				// LLVM 17 doesn't have the no-verify-fixpoint flag.
+				passes = "instcombine"
+			}
 			passOptions := llvm.NewPassBuilderOptions()
 			defer passOptions.Dispose()
-			err = mod.RunPasses("instcombine", llvm.TargetMachine{}, passOptions)
+			err = mod.RunPasses(passes, llvm.TargetMachine{}, passOptions)
 			if err != nil {
 				t.Error(err)
 			}
@@ -324,6 +335,9 @@ func TestCompilerErrors(t *testing.T) {
 		}
 		expectedErrorsIdx++
 	}
+	for _, missing := range expectedErrors[expectedErrorsIdx:] {
+		t.Errorf("expected compiler error was not produced: %s", missing)
+	}
 }
 
 func TestAggregateValueCount(t *testing.T) {

compiler/interface.go

@@ -1298,11 +1298,39 @@ func (c *compilerContext) getInterfaceInvokeWrapper(fn *ssa.Function, llvmFnType
 		return wrapper
 	}
 
-	// Get the expanded receiver type.
+	// Get the expanded receiver type. Exported methods keep their C ABI and
+	// therefore pass even large value receivers directly.
+	exported := c.getFunctionInfo(fn).exported
+
+	// Interface invoke thunks use the Go internal calling convention, but
+	// exported methods keep their C ABI. The receiver is bridged below; a
+	// large aggregate result or parameter, however, is placed differently by
+	// the two conventions (indirect vs. direct), so such a method cannot be
+	// called through an interface. Report that instead of building a wrapper
+	// with a mismatched signature.
+	if exported {
+		if _, indirect := c.hasIndirectResult(fn.Signature); indirect {
+			c.addError(fn.Pos(), fmt.Sprintf("exported method %s with a large aggregate result cannot be called through an interface", fn.RelString(nil)))
+			return llvmFn
+		}
+		for param := range fn.Signature.Params().Variables() {
+			if c.paramNeedsSpill(c.getLLVMType(param.Type())) {
+				c.addError(fn.Pos(), fmt.Sprintf("exported method %s with a large aggregate parameter cannot be called through an interface", fn.RelString(nil)))
+				return llvmFn
+			}
+		}
+	}
+
 	receiverType := c.getLLVMType(fn.Signature.Recv().Type())
 	var expandedReceiverType []llvm.Type
-	receiverIndirect := c.isIndirectAggregate(receiverType)
-	for _, info := range c.expandFormalParamType(receiverType, "", nil) {
+	receiverIndirect := c.isIndirectParam(receiverType, exported)
+	var receiverInfos []paramInfo
+	if exported {
+		receiverInfos = c.expandDirectFormalParamType(receiverType, "", nil)
+	} else {
+		receiverInfos = c.expandFormalParamType(receiverType, "", nil)
+	}
+	for _, info := range receiverInfos {
 		expandedReceiverType = append(expandedReceiverType, info.llvmType)
 	}
 
@@ -1317,7 +1345,7 @@ func (c *compilerContext) getInterfaceInvokeWrapper(fn *ssa.Function, llvmFnType
 
 	// create wrapper function
 	resultOffset := 0
-	if _, indirect := c.hasIndirectResult(fn.Signature); indirect {
+	if _, indirect := c.hasIndirectResult(fn.Signature); indirect && !exported {
 		resultOffset = 1
 	}
 	paramTypes := append([]llvm.Type{}, llvmFnType.ParamTypes()[:resultOffset]...)

compiler/testdata/errors.go

@@ -105,3 +105,34 @@ func invalidreturn_chan_int() chan int
 //
 //go:wasmimport modulename invalidreturn_string
 func invalidreturn_string() string
+
+// Exported methods keep the C calling convention, but interface invoke thunks
+// use the Go internal convention. When a method's result or a parameter is a
+// large aggregate the two conventions place values differently, so calling
+// such a method through an interface is not supported and must be reported
+// instead of silently miscompiled.
+
+type exportedBigResult struct{}
+
+// ERROR: exported method (main.exportedBigResult).makeBig with a large aggregate result cannot be called through an interface
+//
+//export makeBigExported
+func (exportedBigResult) makeBig() [1025]byte {
+	return [1025]byte{}
+}
+
+type exportedBigParam struct{}
+
+// ERROR: exported method (main.exportedBigParam).takeBig with a large aggregate parameter cannot be called through an interface
+//
+//export takeBigExported
+func (exportedBigParam) takeBig(a [17]int32) int32 {
+	return a[16]
+}
+
+func useExportedMethodsThroughInterfaces() {
+	var m interface{ makeBig() [1025]byte } = exportedBigResult{}
+	m.makeBig()
+	var t interface{ takeBig([17]int32) int32 } = exportedBigParam{}
+	t.takeBig([17]int32{})
+}

compiler/testdata/goroutine-wasm-asyncify.ll

@@ -16,7 +16,7 @@ entry:
 ; Function Attrs: nounwind
 define hidden void @main.regularFunctionGoroutine(ptr %context) unnamed_addr #1 {
 entry:
-  call void @"internal/task.start"(i32 ptrtoint (ptr @"main.regularFunction$gowrapper" to i32), ptr nonnull inttoptr (i32 5 to ptr), i32 65536, ptr undef) #11
+  call void @"internal/task.start"(i32 ptrtoint (ptr @"main.regularFunction$gowrapper" to i32), ptr nonnull inttoptr (i32 5 to ptr), i32 131072, ptr undef) #11
   ret void
 }
 
@@ -38,7 +38,7 @@ declare void @"internal/task.start"(i32, ptr, i32, ptr) #0
 ; Function Attrs: nounwind
 define hidden void @main.inlineFunctionGoroutine(ptr %context) unnamed_addr #1 {
 entry:
-  call void @"internal/task.start"(i32 ptrtoint (ptr @"main.inlineFunctionGoroutine$1$gowrapper" to i32), ptr nonnull inttoptr (i32 5 to ptr), i32 65536, ptr undef) #11
+  call void @"internal/task.start"(i32 ptrtoint (ptr @"main.inlineFunctionGoroutine$1$gowrapper" to i32), ptr nonnull inttoptr (i32 5 to ptr), i32 131072, ptr undef) #11
   ret void
 }
 
@@ -71,7 +71,7 @@ entry:
   store i32 5, ptr %0, align 4
   %1 = getelementptr inbounds nuw i8, ptr %0, i32 4
   store ptr %n, ptr %1, align 4
-  call void @"internal/task.start"(i32 ptrtoint (ptr @"main.closureFunctionGoroutine$1$gowrapper" to i32), ptr nonnull %0, i32 65536, ptr undef) #11
+  call void @"internal/task.start"(i32 ptrtoint (ptr @"main.closureFunctionGoroutine$1$gowrapper" to i32), ptr nonnull %0, i32 131072, ptr undef) #11
   %2 = load i32, ptr %n, align 4
   call void @runtime.printlock(ptr undef) #11
   call void @runtime.printint32(i32 %2, ptr undef) #11
@@ -117,7 +117,7 @@ entry:
   store ptr %fn.context, ptr %1, align 4
   %2 = getelementptr inbounds nuw i8, ptr %0, i32 8
   store ptr %fn.funcptr, ptr %2, align 4
-  call void @"internal/task.start"(i32 ptrtoint (ptr @main.funcGoroutine.gowrapper to i32), ptr nonnull %0, i32 65536, ptr undef) #11
+  call void @"internal/task.start"(i32 ptrtoint (ptr @main.funcGoroutine.gowrapper to i32), ptr nonnull %0, i32 131072, ptr undef) #11
   ret void
 }
 
@@ -176,7 +176,7 @@ entry:
   store i32 4, ptr %2, align 4
   %3 = getelementptr inbounds nuw i8, ptr %0, i32 12
   store ptr %itf.typecode, ptr %3, align 4
-  call void @"internal/task.start"(i32 ptrtoint (ptr @"interface:{Print:func:{basic:string}{}}.Print$invoke$gowrapper" to i32), ptr nonnull %0, i32 65536, ptr undef) #11
+  call void @"internal/task.start"(i32 ptrtoint (ptr @"interface:{Print:func:{basic:string}{}}.Print$invoke$gowrapper" to i32), ptr nonnull %0, i32 131072, ptr undef) #11
   ret void
 }
 

compiler/testdata/large.ll

@@ -199,7 +199,7 @@ entry:
   %go.param = call align 1 dereferenceable(1025) ptr @runtime.alloc(i32 1025, ptr nonnull inttoptr (i32 3 to ptr), ptr undef) #9
   call void @runtime.trackPointer(ptr nonnull %go.param, ptr nonnull %stackalloc, ptr undef) #9
   call void @llvm.memcpy.p0.p0.i32(ptr noundef nonnull align 1 dereferenceable(1025) %go.param, ptr noundef nonnull align 1 dereferenceable(1025) %value, i32 1025, i1 false)
-  call void @"internal/task.start"(i32 ptrtoint (ptr @"main.readLargeValue$gowrapper" to i32), ptr nonnull %go.param, i32 65536, ptr undef) #9
+  call void @"internal/task.start"(i32 ptrtoint (ptr @"main.readLargeValue$gowrapper" to i32), ptr nonnull %go.param, i32 131072, ptr undef) #9
   ret void
 }