In go.dev/cl/517775, I moved the frontend's deadcode elimination pass
into unified IR. But I also made a small enhancement: a branch like
"if x || true" is now detected as always taken, so the else branch can
be eliminated.
However, the inliner also has an optimization for delaying the
introduction of the result temporary variables when there's a single
return statement (added in go.dev/cl/266199). Consequently, the
inliner turns "if x || true { return true }; return true" into:
if x || true {
~R0 := true
goto .i0
}
.i0:
// code that uses ~R0
In turn, this confuses phi insertion, because it doesn't recognize
that the "if" statement is always taken, and so ~R0 will always be
initialized.
With this CL, after inlining we instead produce:
_ = x || true
~R0 := true
goto .i0
.i0:
Fixes #62211.
Change-Id: Ic8a12c9eb85833ee4e5d114f60e6c47817fce538
Reviewed-on: https://go-review.googlesource.com/c/go/+/522096
Reviewed-by: Than McIntosh <thanm@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
// @@@ Statements
func (r *reader) stmt() ir.Node {
// @@@ Statements
func (r *reader) stmt() ir.Node {
- switch stmts := r.stmts(); len(stmts) {
+ return block(r.stmts())
+}
+
+func block(stmts []ir.Node) ir.Node {
+ switch len(stmts) {
case 0:
return nil
case 1:
case 0:
return nil
case 1:
-func (r *reader) stmts() []ir.Node {
+func (r *reader) stmts() ir.Nodes {
assert(ir.CurFunc == r.curfn)
var res ir.Nodes
assert(ir.CurFunc == r.curfn)
var res ir.Nodes
pos := r.pos()
init := r.stmts()
cond := r.expr()
pos := r.pos()
init := r.stmts()
cond := r.expr()
- then := r.blockStmt()
- els := r.stmts()
+ staticCond := r.Int()
+ var then, els []ir.Node
+ if staticCond >= 0 {
+ then = r.blockStmt()
+ } else {
+ r.lastCloseScopePos = r.pos()
+ }
+ if staticCond <= 0 {
+ els = r.stmts()
+ }
- if ir.IsConst(cond, constant.Bool) && len(init)+len(then)+len(els) == 0 {
- return nil // drop empty if statement
+ if staticCond != 0 {
+ // We may have removed a dead return statement, which can trip up
+ // later passes (#62211). To avoid confusion, we instead flatten
+ // the if statement into a block.
+
+ if cond.Op() != ir.OLITERAL {
+ init.Append(typecheck.Stmt(ir.NewAssignStmt(pos, ir.BlankNode, cond))) // for side effects
+ }
+ init.Append(then...)
+ init.Append(els...)
+ return block(init)
}
n := ir.NewIfStmt(pos, cond, then, els)
}
n := ir.NewIfStmt(pos, cond, then, els)
}
func (w *writer) ifStmt(stmt *syntax.IfStmt) {
}
func (w *writer) ifStmt(stmt *syntax.IfStmt) {
- switch cond := w.p.staticBool(&stmt.Cond); {
- case cond > 0: // always true
- stmt.Else = nil
- case cond < 0: // always false
- stmt.Then.List = nil
- }
+ cond := w.p.staticBool(&stmt.Cond)
w.Sync(pkgbits.SyncIfStmt)
w.openScope(stmt.Pos())
w.pos(stmt)
w.stmt(stmt.Init)
w.expr(stmt.Cond)
w.Sync(pkgbits.SyncIfStmt)
w.openScope(stmt.Pos())
w.pos(stmt)
w.stmt(stmt.Init)
w.expr(stmt.Cond)
- w.blockStmt(stmt.Then)
- w.stmt(stmt.Else)
+ w.Int(cond)
+ if cond >= 0 {
+ w.blockStmt(stmt.Then)
+ } else {
+ w.pos(stmt.Then.Rbrace)
+ }
+ if cond <= 0 {
+ w.stmt(stmt.Else)
+ }
select2(x, y) // ERROR "inlining call to select2"
}
}
select2(x, y) // ERROR "inlining call to select2"
}
}
+
+// Issue #62211: inlining a function with unreachable "return"
+// statements could trip up phi insertion.
+func issue62211(x bool) { // ERROR "can inline issue62211"
+ if issue62211F(x) { // ERROR "inlining call to issue62211F"
+ }
+ if issue62211G(x) { // ERROR "inlining call to issue62211G"
+ }
+
+ // Initial fix CL caused a "non-monotonic scope positions" failure
+ // on code like this.
+ if z := 0; false {
+ panic(z)
+ }
+}
+
+func issue62211F(x bool) bool { // ERROR "can inline issue62211F"
+ if x || true {
+ return true
+ }
+ return true
+}
+
+func issue62211G(x bool) bool { // ERROR "can inline issue62211G"
+ if x || true {
+ return true
+ } else {
+ return true
+ }
+}