wasm: fix GC scanning of allocas

Scanning of allocas was entirely broken on WebAssembly. The code
intended to do this was never run. There were also no tests.

Looking into this further, I found that it is actually not really
necessary to do that: the C stack can be scanned conservatively and in
fact this was already done for goroutine stacks (because they live on
the heap and are always referenced). It wasn't done for the system stack
however.

With these fixes, I believe code should be both faster *and* more
correct.

I found this in my work to get opaque pointers supported in LLVM 15,
because the code that was never reached now finally got run and was
actually quite buggy.

2 files changed, 16 insertions, 42 deletions

diff --git a/transform/gc.go b/transform/gc.go
index eb3520aa4..87dc6e88a 100644
--- a/transform/gc.go
+++ b/transform/gc.go
@@ -139,7 +139,7 @@ func MakeGCStackSlots(mod llvm.Module) bool {
 		}
 
 		// Determine what to do with each call.
-		var allocas, pointers []llvm.Value
+		var pointers []llvm.Value
 		for _, call := range calls {
 			ptr := call.Operand(0)
 			call.EraseFromParentAsInstruction()
@@ -189,16 +189,15 @@ func MakeGCStackSlots(mod llvm.Module) bool {
 				// be optimized if needed.
 			}
 
-			if !ptr.IsAAllocaInst().IsNil() {
-				if typeHasPointers(ptr.Type().ElementType()) {
-					allocas = append(allocas, ptr)
-				}
-			} else {
-				pointers = append(pointers, ptr)
+			if ptr := stripPointerCasts(ptr); !ptr.IsAAllocaInst().IsNil() {
+				// Allocas don't need to be tracked because they are allocated
+				// on the C stack which is scanned separately.
+				continue
 			}
+			pointers = append(pointers, ptr)
 		}
 
-		if len(allocas) == 0 && len(pointers) == 0 {
+		if len(pointers) == 0 {
 			// This function does not need to keep track of stack pointers.
 			continue
 		}
@@ -208,9 +207,6 @@ func MakeGCStackSlots(mod llvm.Module) bool {
 			stackChainStartType, // Pointer to parent frame.
 			uintptrType,         // Number of elements in this frame.
 		}
-		for _, alloca := range allocas {
-			fields = append(fields, alloca.Type().ElementType())
-		}
 		for _, ptr := range pointers {
 			fields = append(fields, ptr.Type())
 		}
@@ -235,16 +231,6 @@ func MakeGCStackSlots(mod llvm.Module) bool {
 		stackObjectCast := builder.CreateBitCast(stackObject, stackChainStartType, "")
 		builder.CreateStore(stackObjectCast, stackChainStart)
 
-		// Replace all independent allocas with GEPs in the stack object.
-		for i, alloca := range allocas {
-			gep := builder.CreateGEP(stackObject, []llvm.Value{
-				llvm.ConstInt(ctx.Int32Type(), 0, false),
-				llvm.ConstInt(ctx.Int32Type(), uint64(2+i), false),
-			}, "")
-			alloca.ReplaceAllUsesWith(gep)
-			alloca.EraseFromParentAsInstruction()
-		}
-
 		// Do a store to the stack object after each new pointer that is created.
 		pointerStores := make(map[llvm.Value]struct{})
 		for i, ptr := range pointers {
@@ -260,7 +246,7 @@ func MakeGCStackSlots(mod llvm.Module) bool {
 			// Extract a pointer to the appropriate section of the stack object.
 			gep := builder.CreateGEP(stackObject, []llvm.Value{
 				llvm.ConstInt(ctx.Int32Type(), 0, false),
-				llvm.ConstInt(ctx.Int32Type(), uint64(2+len(allocas)+i), false),
+				llvm.ConstInt(ctx.Int32Type(), uint64(2+i), false),
 			}, "")
 
 			// Store the pointer into the stack slot.
diff --git a/transform/llvm.go b/transform/llvm.go
index 90b7a7c75..32ee95604 100644
--- a/transform/llvm.go
+++ b/transform/llvm.go
@@ -75,26 +75,14 @@ func replaceGlobalIntWithArray(mod llvm.Module, name string, buf interface{}) ll
 	return global
 }
 
-// typeHasPointers returns whether this type is a pointer or contains pointers.
-// If the type is an aggregate type, it will check whether there is a pointer
-// inside.
-func typeHasPointers(t llvm.Type) bool {
-	switch t.TypeKind() {
-	case llvm.PointerTypeKind:
-		return true
-	case llvm.StructTypeKind:
-		for _, subType := range t.StructElementTypes() {
-			if typeHasPointers(subType) {
-				return true
-			}
+// stripPointerCasts strips instruction pointer casts (getelementptr and
+// bitcast) and returns the original value without the casts.
+func stripPointerCasts(value llvm.Value) llvm.Value {
+	if !value.IsAInstruction().IsNil() {
+		switch value.InstructionOpcode() {
+		case llvm.GetElementPtr, llvm.BitCast:
+			return stripPointerCasts(value.Operand(0))
 		}
-		return false
-	case llvm.ArrayTypeKind:
-		if typeHasPointers(t.ElementType()) {
-			return true
-		}
-		return false
-	default:
-		return false
 	}
+	return value
 }