Skip to content

Commit dcaf3fb

Browse files
isharipoTocarIP
isharipo
authored andcommitted
cmd/compile: make DCE remove nodes after terminating if
This change makes compiler frontend dead code elimination of const expr if statements introduced in https://golang.org/cl/38773 treat both if constCondTrue { ...; returnStmt } toBeRemoved... if constCondFalse { ...; } else { returnStmt } toBeRemoved... identically to: if constCondTrue { ...; returnStmt } else { toBeRemoved... } Where "constCondTrue" is a an expression that can be evaluated to "true" during compile time. The additional checks are only triggered for const expr if conditions that evaluate to true. name old time/op new time/op delta Template 431ms ± 2% 429ms ± 1% ~ (p=0.491 n=8+6) Unicode 198ms ± 4% 201ms ± 2% ~ (p=0.234 n=7+6) GoTypes 1.40s ± 1% 1.41s ± 2% ~ (p=0.053 n=7+7) Compiler 6.72s ± 2% 6.81s ± 1% +1.35% (p=0.011 n=7+7) SSA 17.3s ± 1% 17.3s ± 2% ~ (p=0.731 n=6+7) Flate 275ms ± 2% 275ms ± 2% ~ (p=0.902 n=7+7) GoParser 340ms ± 2% 339ms ± 2% ~ (p=0.902 n=7+7) Reflect 910ms ± 2% 905ms ± 1% ~ (p=0.310 n=6+6) Tar 403ms ± 1% 403ms ± 2% ~ (p=0.366 n=7+6) XML 486ms ± 1% 490ms ± 1% ~ (p=0.065 n=6+6) StdCmd 56.2s ± 1% 56.6s ± 2% ~ (p=0.620 n=7+7) name old user-time/op new user-time/op delta Template 559ms ± 8% 557ms ± 7% ~ (p=0.713 n=8+7) Unicode 266ms ±13% 277ms ± 9% ~ (p=0.157 n=8+7) GoTypes 1.83s ± 2% 1.84s ± 1% ~ (p=0.522 n=8+7) Compiler 8.67s ± 4% 8.89s ± 4% ~ (p=0.077 n=7+7) SSA 23.9s ± 1% 24.2s ± 1% +1.31% (p=0.005 n=7+7) Flate 351ms ± 4% 342ms ± 5% ~ (p=0.105 n=7+7) GoParser 437ms ± 2% 423ms ± 5% -3.14% (p=0.016 n=7+7) Reflect 1.16s ± 3% 1.15s ± 2% ~ (p=0.362 n=7+7) Tar 517ms ± 4% 511ms ± 3% ~ (p=0.538 n=7+7) XML 619ms ± 3% 617ms ± 4% ~ (p=0.483 n=7+7) Fixes #23521 Change-Id: I165a7827d869aeb93ce6047d026ff873d039a4f3 Reviewed-on: https://go-review.googlesource.com/91056 Run-TryBot: Iskander Sharipov <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Josh Bleecher Snyder <[email protected]>
1 parent 19ee2ef commit dcaf3fb

File tree

2 files changed

+65
-1
lines changed

2 files changed

+65
-1
lines changed

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

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3926,24 +3926,45 @@ func deadcode(fn *Node) {
39263926
}
39273927

39283928
func deadcodeslice(nn Nodes) {
3929-
for _, n := range nn.Slice() {
3929+
for i, n := range nn.Slice() {
3930+
// Cut is set to true when all nodes after i'th position
3931+
// should be removed.
3932+
// In other words, it marks whole slice "tail" as dead.
3933+
cut := false
39303934
if n == nil {
39313935
continue
39323936
}
39333937
if n.Op == OIF {
39343938
n.Left = deadcodeexpr(n.Left)
39353939
if Isconst(n.Left, CTBOOL) {
3940+
var body Nodes
39363941
if n.Left.Bool() {
39373942
n.Rlist = Nodes{}
3943+
body = n.Nbody
39383944
} else {
39393945
n.Nbody = Nodes{}
3946+
body = n.Rlist
3947+
}
3948+
// If "then" or "else" branch ends with panic or return statement,
3949+
// it is safe to remove all statements after this node.
3950+
// isterminating is not used to avoid goto-related complications.
3951+
if body := body.Slice(); len(body) != 0 {
3952+
switch body[(len(body) - 1)].Op {
3953+
case ORETURN, ORETJMP, OPANIC:
3954+
cut = true
3955+
}
39403956
}
39413957
}
39423958
}
3959+
39433960
deadcodeslice(n.Ninit)
39443961
deadcodeslice(n.Nbody)
39453962
deadcodeslice(n.List)
39463963
deadcodeslice(n.Rlist)
3964+
if cut {
3965+
*nn.slice = nn.Slice()[:i+1]
3966+
break
3967+
}
39473968
}
39483969
}
39493970

test/fixedbugs/issue23521.go

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
// errorcheck -0 -m
2+
3+
// Copyright 2018 The Go Authors. All rights reserved.
4+
// Use of this source code is governed by a BSD-style
5+
// license that can be found in the LICENSE file.
6+
7+
// Issue 23521: improve early DCE for if without explicit else.
8+
9+
package p
10+
11+
//go:noinline
12+
func nonleaf() {}
13+
14+
const truth = true
15+
16+
func f() int { // ERROR "can inline f"
17+
if truth {
18+
return 0
19+
}
20+
// If everything below is removed, as it should,
21+
// function f should be inlineable.
22+
nonleaf()
23+
for {
24+
panic("")
25+
}
26+
}
27+
28+
func g() int { // ERROR "can inline g"
29+
return f() // ERROR "inlining call to f"
30+
}
31+
32+
func f2() int { // ERROR "can inline f2"
33+
if !truth {
34+
nonleaf()
35+
} else {
36+
return 0
37+
}
38+
panic("")
39+
}
40+
41+
func g2() int { // ERROR "can inline g2"
42+
return f2() // ERROR "inlining call to f2"
43+
}

0 commit comments

Comments
 (0)