]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/compile: enable more inlining for unified IR
authorMatthew Dempsky <mdempsky@google.com>
Thu, 18 Aug 2022 12:20:16 +0000 (05:20 -0700)
committerMatthew Dempsky <mdempsky@google.com>
Thu, 18 Aug 2022 17:26:40 +0000 (17:26 +0000)
The non-unified frontend had repeated issues with inlining and
generics (#49309, #51909, #52907), which led us to substantially
restrict inlining when shape types were present.

However, these issues are evidently not present in unified IR's
inliner, and the safety restrictions added for the non-unified
frontend can simply be disabled in unified mode.

Fixes #54497.

Change-Id: I8e6ac9f3393c588bfaf14c6452891b9640a9d1bd
Reviewed-on: https://go-review.googlesource.com/c/go/+/424775
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
src/cmd/compile/internal/inline/inl.go
test/run.go
test/typeparam/issue49309.go
test/typeparam/issue54497.go [new file with mode: 0644]

index 795486f7a2c992b9b9ffa110451347236c120f25..ce667d3bed76cda69caa3b50513a700daff9abaf 100644 (file)
@@ -722,44 +722,47 @@ func mkinlcall(n *ir.CallExpr, fn *ir.Func, maxCost int32, inlMap map[*ir.Func]b
                return n
        }
 
-       // Don't inline a function fn that has no shape parameters, but is passed at
-       // least one shape arg. This means we must be inlining a non-generic function
-       // fn that was passed into a generic function, and can be called with a shape
-       // arg because it matches an appropriate type parameters. But fn may include
-       // an interface conversion (that may be applied to a shape arg) that was not
-       // apparent when we first created the instantiation of the generic function.
-       // We can't handle this if we actually do the inlining, since we want to know
-       // all interface conversions immediately after stenciling. So, we avoid
-       // inlining in this case, see issue #49309. (1)
-       //
-       // See discussion on go.dev/cl/406475 for more background.
-       if !fn.Type().Params().HasShape() {
-               for _, arg := range n.Args {
-                       if arg.Type().HasShape() {
+       // The non-unified frontend has issues with inlining and shape parameters.
+       if base.Debug.Unified == 0 {
+               // Don't inline a function fn that has no shape parameters, but is passed at
+               // least one shape arg. This means we must be inlining a non-generic function
+               // fn that was passed into a generic function, and can be called with a shape
+               // arg because it matches an appropriate type parameters. But fn may include
+               // an interface conversion (that may be applied to a shape arg) that was not
+               // apparent when we first created the instantiation of the generic function.
+               // We can't handle this if we actually do the inlining, since we want to know
+               // all interface conversions immediately after stenciling. So, we avoid
+               // inlining in this case, see issue #49309. (1)
+               //
+               // See discussion on go.dev/cl/406475 for more background.
+               if !fn.Type().Params().HasShape() {
+                       for _, arg := range n.Args {
+                               if arg.Type().HasShape() {
+                                       if logopt.Enabled() {
+                                               logopt.LogOpt(n.Pos(), "cannotInlineCall", "inline", ir.FuncName(ir.CurFunc),
+                                                       fmt.Sprintf("inlining function %v has no-shape params with shape args", ir.FuncName(fn)))
+                                       }
+                                       return n
+                               }
+                       }
+               } else {
+                       // Don't inline a function fn that has shape parameters, but is passed no shape arg.
+                       // See comments (1) above, and issue #51909.
+                       inlineable := len(n.Args) == 0 // Function has shape in type, with no arguments can always be inlined.
+                       for _, arg := range n.Args {
+                               if arg.Type().HasShape() {
+                                       inlineable = true
+                                       break
+                               }
+                       }
+                       if !inlineable {
                                if logopt.Enabled() {
                                        logopt.LogOpt(n.Pos(), "cannotInlineCall", "inline", ir.FuncName(ir.CurFunc),
-                                               fmt.Sprintf("inlining function %v has no-shape params with shape args", ir.FuncName(fn)))
+                                               fmt.Sprintf("inlining function %v has shape params with no-shape args", ir.FuncName(fn)))
                                }
                                return n
                        }
                }
-       } else {
-               // Don't inline a function fn that has shape parameters, but is passed no shape arg.
-               // See comments (1) above, and issue #51909.
-               inlineable := len(n.Args) == 0 // Function has shape in type, with no arguments can always be inlined.
-               for _, arg := range n.Args {
-                       if arg.Type().HasShape() {
-                               inlineable = true
-                               break
-                       }
-               }
-               if !inlineable {
-                       if logopt.Enabled() {
-                               logopt.LogOpt(n.Pos(), "cannotInlineCall", "inline", ir.FuncName(ir.CurFunc),
-                                       fmt.Sprintf("inlining function %v has shape params with no-shape args", ir.FuncName(fn)))
-                       }
-                       return n
-               }
        }
 
        if base.Flag.Cfg.Instrumenting && types.IsRuntimePkg(fn.Sym().Pkg) {
index cdbe15c3890269acfbc36ccb5e67594fc0756d15..21f0f7d634cec6df073585a62b3ab1b0a6d084b4 100644 (file)
@@ -1990,6 +1990,7 @@ var go118Failures = setOf(
        "fixedbugs/issue54343.go",  // 1.18 compiler assigns receiver parameter to global variable
        "typeparam/nested.go",      // 1.18 compiler doesn't support function-local types with generics
        "typeparam/issue51521.go",  // 1.18 compiler produces bad panic message and link error
+       "typeparam/issue54497.go",  // 1.18 compiler is more conservative about inlining due to repeated issues
        "typeparam/mdempsky/16.go", // 1.18 compiler uses interface shape type in failed type assertions
        "typeparam/mdempsky/17.go", // 1.18 compiler mishandles implicit conversions from range loops
        "typeparam/mdempsky/18.go", // 1.18 compiler mishandles implicit conversions in select statements
index 265e0bf525bf933a0b8e1042edcdcf43e75fc576..16c97cd451a2156c66ddeb2f1bdfe060995a31a9 100644 (file)
@@ -18,6 +18,7 @@ func myfunc(c string) {
 
 //go:noinline
 func test2(a interface{}) {
+       _ = a.(string)
 }
 
 func main() {
diff --git a/test/typeparam/issue54497.go b/test/typeparam/issue54497.go
new file mode 100644 (file)
index 0000000..1b24cd9
--- /dev/null
@@ -0,0 +1,19 @@
+// errorcheck -0 -m
+
+// Copyright 2022 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.
+
+// Test that inlining works with generic functions.
+
+package testcase
+
+type C interface{ ~uint | ~uint32 | ~uint64 }
+
+func isAligned[T C](x, y T) bool { // ERROR "can inline isAligned\[uint\]" "can inline isAligned\[go\.shape\.uint\]" "inlining call to isAligned\[go\.shape\.uint\]"
+       return x%y == 0
+}
+
+func foo(x uint) bool { // ERROR "can inline foo"
+       return isAligned(x, 64) // ERROR "inlining call to isAligned\[go\.shape\.uint\]"
+}