From 5d6581d74796ac748441c94e84eefdaf338d266c Mon Sep 17 00:00:00 2001
From: Cherry Zhang <cherryyz@google.com>
Date: Wed, 31 Mar 2021 12:41:20 -0400
Subject: [PATCH] cmd/compile: deduplicate OpArg's across types

For in-register arguments, it must have only a single copy of it
present in the function. If there are multiple copies, it confuses
the register allocator, as they are in the same register.

Change-Id: I55cb06746f08aa7c9168026d0f411bce0a9f93f4
Reviewed-on: https://go-review.googlesource.com/c/go/+/306330
Trust: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
---
 src/cmd/compile/internal/ssa/expand_calls.go | 37 ++++++++++++++-
 test/abi/defer_aggregate.go                  | 48 ++++++++++++++++++++
 2 files changed, 84 insertions(+), 1 deletion(-)
 create mode 100644 test/abi/defer_aggregate.go

diff --git a/src/cmd/compile/internal/ssa/expand_calls.go b/src/cmd/compile/internal/ssa/expand_calls.go
index b6aba0ed16..2935236473 100644
--- a/src/cmd/compile/internal/ssa/expand_calls.go
+++ b/src/cmd/compile/internal/ssa/expand_calls.go
@@ -1359,7 +1359,42 @@ func expandCalls(f *Func) {
 		}
 	}
 
-	// Step 5: elide any copies introduced.
+	// Step 5: dedup OpArgXXXReg values. Mostly it is already dedup'd by commonArgs,
+	// but there are cases that we have same OpArgXXXReg values with different types.
+	// E.g. string is sometimes decomposed as { *int8, int }, sometimes as { unsafe.Pointer, uintptr }.
+	// (Can we avoid that?)
+	var IArg, FArg [32]*Value
+	for _, v := range f.Entry.Values {
+		switch v.Op {
+		case OpArgIntReg:
+			i := v.AuxInt
+			if w := IArg[i]; w != nil {
+				if w.Type.Width != v.Type.Width {
+					f.Fatalf("incompatible OpArgIntReg [%d]: %v and %v", i, v, w)
+				}
+				if w.Type.IsUnsafePtr() && !v.Type.IsUnsafePtr() {
+					// Update unsafe.Pointer type if we know the actual pointer type.
+					w.Type = v.Type
+				}
+				// TODO: don't dedup pointer and scalar? Rewrite to OpConvert? Can it happen?
+				v.copyOf(w)
+			} else {
+				IArg[i] = v
+			}
+		case OpArgFloatReg:
+			i := v.AuxInt
+			if w := FArg[i]; w != nil {
+				if w.Type.Width != v.Type.Width {
+					f.Fatalf("incompatible OpArgFloatReg [%d]: %v and %v", i, v, w)
+				}
+				v.copyOf(w)
+			} else {
+				FArg[i] = v
+			}
+		}
+	}
+
+	// Step 6: elide any copies introduced.
 	for _, b := range f.Blocks {
 		for _, v := range b.Values {
 			for i, a := range v.Args {
diff --git a/test/abi/defer_aggregate.go b/test/abi/defer_aggregate.go
new file mode 100644
index 0000000000..6dd82828c1
--- /dev/null
+++ b/test/abi/defer_aggregate.go
@@ -0,0 +1,48 @@
+// run
+
+// Copyright 2021 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package main
+
+const p0exp = "foo"
+const p1exp = 10101
+const p2exp = 3030303
+const p3exp = 505050505
+const p4exp = 70707070707
+
+//go:noinline
+//go:registerparams
+func callee(p0 string, p1 uint64, p2 uint64, p3 uint64, p4 uint64) {
+	if p0 != p0exp {
+		panic("bad p0")
+	}
+	if p1 != p1exp {
+		panic("bad p1")
+	}
+	if p2 != p2exp {
+		panic("bad p2")
+	}
+	if p3 != p3exp {
+		panic("bad p3")
+	}
+	if p4 != p4exp {
+		panic("bad p4")
+	}
+	defer func(p0 string, p2 uint64) {
+		if p0 != p0exp {
+			panic("defer bad p0")
+		}
+		if p1 != p1exp {
+			panic("defer bad p1")
+		}
+		if p2 != p2exp {
+			panic("defer bad p2")
+		}
+	}(p0, p2)
+}
+
+func main() {
+	callee(p0exp, p1exp, p2exp, p3exp, p4exp)
+}