cmd/compile: allow inlining of "for" loops

We already allow inlining "if" and "goto" statements, so we might as
well allow "for" loops too. The majority of frontend support is
already there too.

The critical missing feature at the moment is that inlining doesn't
properly reassociate OLABEL nodes with their control statement (e.g.,
OFOR) after inlining. This eventually causes SSA construction to fail.

As a workaround, this CL only enables inlining for unlabeled "for"
loops. It's left to a (yet unplanned) future CL to add support for
labeled "for" loops.

The increased opportunity for inlining leads to a small growth in
binary size. For example:

$ size go.old go.new
   text	   data	    bss	    dec	    hex	filename
9740163	 320064	 230656	10290883	 9d06c3	go.old
9793399	 320064	 230656	10344119	 9dd6b7	go.new

Updates #14768.
Fixes #41474.

Change-Id: I827db0b2b9d9fa2934db05caf6baa463f0cd032a
Reviewed-on: https://go-review.googlesource.com/c/go/+/256459
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>

diff --git a/src/cmd/compile/internal/gc/inl.go b/src/cmd/compile/internal/gc/inl.go
index cac51685dff7917e7fc99fea9c8e6f666e573d34..8630560a9afaa04f0636acac145e152396859f82 100644 (file)
--- a/src/cmd/compile/internal/gc/inl.go
+++ b/src/cmd/compile/internal/gc/inl.go
@@ -385,14 +385,11 @@ func (v *hairyVisitor) visit(n *Node) bool {
        case OCLOSURE,
                OCALLPART,
                ORANGE,
-               OFOR,
-               OFORUNTIL,
                OSELECT,
                OTYPESW,
                OGO,
                ODEFER,
                ODCLTYPE, // can't print yet
-               OBREAK,
                ORETJMP:
                v.reason = "unhandled op " + n.Op.String()
                return true
@@ -400,10 +397,23 @@ func (v *hairyVisitor) visit(n *Node) bool {
        case OAPPEND:
                v.budget -= inlineExtraAppendCost
 
-       case ODCLCONST, OEMPTY, OFALL, OLABEL:
+       case ODCLCONST, OEMPTY, OFALL:
                // These nodes don't produce code; omit from inlining budget.
                return false
 
+       case OLABEL:
+               // TODO(mdempsky): Add support for inlining labeled control statements.
+               if n.labeledControl() != nil {
+                       v.reason = "labeled control"
+                       return true
+               }
+
+       case OBREAK, OCONTINUE:
+               if n.Sym != nil {
+                       // Should have short-circuited due to labeledControl above.
+                       Fatalf("unexpected labeled break/continue: %v", n)
+               }
+
        case OIF:
                if Isconst(n.Left, CTBOOL) {
                        // This if and the condition cost nothing.

diff --git a/test/closure3.dir/main.go b/test/closure3.dir/main.go
index 3ec90139a37b24ed72659b523bf83ddc172142d9..5694673f1ec64bece849d7473362fb4296f03ff7 100644 (file)
--- a/test/closure3.dir/main.go
+++ b/test/closure3.dir/main.go
@@ -238,8 +238,7 @@ func main() {
                                if c != 4 {
                                        ppanic("c != 4")
                                }
-                               for i := 0; i < 10; i++ { // prevent inlining
-                               }
+                               recover() // prevent inlining
                        }()
                }()
                if c != 4 {

diff --git a/test/inline.go b/test/inline.go
index 3edcf2edfd128bf88f9897de18c8a9269d69508c..2f6fc0fe88066a86a05f475b7a3353d182704e0b 100644 (file)
--- a/test/inline.go
+++ b/test/inline.go
@@ -197,3 +197,26 @@ func gg(x int) { // ERROR "can inline gg"
 func hh(x int) { // ERROR "can inline hh"
        ff(x - 1) // ERROR "inlining call to ff"  // ERROR "inlining call to gg"
 }
+
+// Issue #14768 - make sure we can inline for loops.
+func for1(fn func() bool) { // ERROR "can inline for1" "fn does not escape"
+       for {
+               if fn() {
+                       break
+               } else {
+                       continue
+               }
+       }
+}
+
+// BAD: for2 should be inlineable too.
+func for2(fn func() bool) { // ERROR "fn does not escape"
+Loop:
+       for {
+               if fn() {
+                       break Loop
+               } else {
+                       continue Loop
+               }
+       }
+}