Skip to content

Commit 497ea06

Browse files
committed
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>
1 parent c0417df commit 497ea06

File tree

3 files changed

+38
-6
lines changed

3 files changed

+38
-6
lines changed

src/cmd/compile/internal/gc/inl.go

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -385,25 +385,35 @@ func (v *hairyVisitor) visit(n *Node) bool {
385385
case OCLOSURE,
386386
OCALLPART,
387387
ORANGE,
388-
OFOR,
389-
OFORUNTIL,
390388
OSELECT,
391389
OTYPESW,
392390
OGO,
393391
ODEFER,
394392
ODCLTYPE, // can't print yet
395-
OBREAK,
396393
ORETJMP:
397394
v.reason = "unhandled op " + n.Op.String()
398395
return true
399396

400397
case OAPPEND:
401398
v.budget -= inlineExtraAppendCost
402399

403-
case ODCLCONST, OEMPTY, OFALL, OLABEL:
400+
case ODCLCONST, OEMPTY, OFALL:
404401
// These nodes don't produce code; omit from inlining budget.
405402
return false
406403

404+
case OLABEL:
405+
// TODO(mdempsky): Add support for inlining labeled control statements.
406+
if n.labeledControl() != nil {
407+
v.reason = "labeled control"
408+
return true
409+
}
410+
411+
case OBREAK, OCONTINUE:
412+
if n.Sym != nil {
413+
// Should have short-circuited due to labeledControl above.
414+
Fatalf("unexpected labeled break/continue: %v", n)
415+
}
416+
407417
case OIF:
408418
if Isconst(n.Left, CTBOOL) {
409419
// This if and the condition cost nothing.

test/closure3.dir/main.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -238,8 +238,7 @@ func main() {
238238
if c != 4 {
239239
ppanic("c != 4")
240240
}
241-
for i := 0; i < 10; i++ { // prevent inlining
242-
}
241+
recover() // prevent inlining
243242
}()
244243
}()
245244
if c != 4 {

test/inline.go

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,3 +197,26 @@ func gg(x int) { // ERROR "can inline gg"
197197
func hh(x int) { // ERROR "can inline hh"
198198
ff(x - 1) // ERROR "inlining call to ff" // ERROR "inlining call to gg"
199199
}
200+
201+
// Issue #14768 - make sure we can inline for loops.
202+
func for1(fn func() bool) { // ERROR "can inline for1" "fn does not escape"
203+
for {
204+
if fn() {
205+
break
206+
} else {
207+
continue
208+
}
209+
}
210+
}
211+
212+
// BAD: for2 should be inlineable too.
213+
func for2(fn func() bool) { // ERROR "fn does not escape"
214+
Loop:
215+
for {
216+
if fn() {
217+
break Loop
218+
} else {
219+
continue Loop
220+
}
221+
}
222+
}

0 commit comments

Comments
 (0)