From 142566e5294c54d01845800a9b3d6bfce81841bb Mon Sep 17 00:00:00 2001 From: Alan Donovan Date: Thu, 12 Jun 2014 11:31:41 -0400 Subject: [PATCH] go/ssa: avoid "premature optimization" of dead branch removal. Blocks dominated by "if false" should be retained in the initial SSA form so they remain visible to subsequent source code analysis tools. In any case, true compilers already need a stronger version of this optimization so they can simplify CFGs such as this: const x, y = ... switch x {case y:...} where a branch is constant but the comparison of constants does not occur within an expression. LGTM=gri R=gri CC=golang-codereviews, pcc https://golang.org/cl/101250043 --- go/ssa/builder.go | 18 +++++++----------- go/ssa/sanity.go | 9 ++++++--- go/ssa/testdata/objlookup.go | 6 +++--- oracle/testdata/src/main/callgraph-json.golden | 1 + oracle/testdata/src/main/callgraph.golden | 2 ++ oracle/testdata/src/main/calls.golden | 3 ++- 6 files changed, 21 insertions(+), 18 deletions(-) diff --git a/go/ssa/builder.go b/go/ssa/builder.go index fcf320edc7..cd3417ea44 100644 --- a/go/ssa/builder.go +++ b/go/ssa/builder.go @@ -106,17 +106,13 @@ func (b *builder) cond(fn *Function, e ast.Expr, t, f *BasicBlock) { } } - switch cond := b.expr(fn, e).(type) { - case *Const: - // Dispatch constant conditions statically. - if exact.BoolVal(cond.Value) { - emitJump(fn, t) - } else { - emitJump(fn, f) - } - default: - emitIf(fn, cond, t, f) - } + // A traditional compiler would simplify "if false" (etc) here + // but we do not, for better fidelity to the source code. + // + // The value of a constant condition may be platform-specific, + // and may cause blocks that are reachable in some configuration + // to be hidden from subsequent analyses such as bug-finding tools. + emitIf(fn, b.expr(fn, e), t, f) } // logicalBinop emits code to fn to evaluate e, a &&- or diff --git a/go/ssa/sanity.go b/go/ssa/sanity.go index 48e967a526..33b5b109df 100644 --- a/go/ssa/sanity.go +++ b/go/ssa/sanity.go @@ -197,9 +197,12 @@ func (s *sanity) checkInstr(idx int, instr Instruction) { } } - // TODO(adonovan): sanity-check Consts used as instruction Operands(), - // e.g. reject Consts with "untyped" types. - // + // Untyped constants are legal as instruction Operands(), + // for example: + // _ = "foo"[0] + // or: + // if wordsize==64 {...} + // All other non-Instruction Values can be found via their // enclosing Function or Package. } diff --git a/go/ssa/testdata/objlookup.go b/go/ssa/testdata/objlookup.go index d2862d33e3..3468a4a50f 100644 --- a/go/ssa/testdata/objlookup.go +++ b/go/ssa/testdata/objlookup.go @@ -112,10 +112,10 @@ func main() { _ = v11.method // v11::Const _ = (*struct{ J }).method // J::nil - // These vars are optimised away. + // These vars are not optimised away. if false { - v13 := 0 // v13::nil - println(v13) // v13::nil + v13 := 0 // v13::Const + println(v13) // v13::Const } switch x := 1; x { // x::Const diff --git a/oracle/testdata/src/main/callgraph-json.golden b/oracle/testdata/src/main/callgraph-json.golden index f0d10860c9..cb6b6bf90c 100644 --- a/oracle/testdata/src/main/callgraph-json.golden +++ b/oracle/testdata/src/main/callgraph-json.golden @@ -6,6 +6,7 @@ "name": "main.main", "pos": "testdata/src/main/callgraph-json.go:24:6", "children": [ + 0, 1, 2, 3 diff --git a/oracle/testdata/src/main/callgraph.golden b/oracle/testdata/src/main/callgraph.golden index 34a942613a..654aa25697 100644 --- a/oracle/testdata/src/main/callgraph.golden +++ b/oracle/testdata/src/main/callgraph.golden @@ -12,6 +12,7 @@ Non-numbered nodes indicate back- or cross-edges to the node whose 4 B 5 call2 6 main$1 + main (1) 7 nop -------- @callgraph callgraph-complete -------- @@ -31,5 +32,6 @@ Non-numbered nodes indicate back- or cross-edges to the node whose 7 main.B 8 main.call2 9 main$1 + main.main (3) 10 main.nop diff --git a/oracle/testdata/src/main/calls.golden b/oracle/testdata/src/main/calls.golden index 96e3b389d5..3cbeb85f25 100644 --- a/oracle/testdata/src/main/calls.golden +++ b/oracle/testdata/src/main/calls.golden @@ -71,8 +71,9 @@ Error: this is a type conversion, not a function call Error: ambiguous selection within function call (or conversion) -------- @callees callees-err-deadcode1 -------- +this static function call dispatches to: + main.main -Error: this call site is unreachable in this analysis -------- @callees callees-err-nil-func -------- dynamic function call on nil value