From 98938189a16a764007bce7fcd4bfeb2386043208 Mon Sep 17 00:00:00 2001 From: Keith Randall Date: Tue, 27 Sep 2016 14:39:27 -0700 Subject: [PATCH] cmd/compile: remove duplicate nilchecks Mark nil check operations as faulting if their arg is zero. This lets the late nilcheck pass remove duplicates. Fixes #17242. Change-Id: I4c9938d8a5a1e43edd85b4a66f0b34004860bcd9 Reviewed-on: https://go-review.googlesource.com/29952 Run-TryBot: Keith Randall TryBot-Result: Gobot Gobot Reviewed-by: Cherry Zhang --- src/cmd/compile/internal/ssa/gen/386Ops.go | 2 +- src/cmd/compile/internal/ssa/gen/AMD64Ops.go | 2 +- src/cmd/compile/internal/ssa/gen/ARM64Ops.go | 2 +- src/cmd/compile/internal/ssa/gen/ARMOps.go | 2 +- src/cmd/compile/internal/ssa/gen/MIPS64Ops.go | 2 +- src/cmd/compile/internal/ssa/gen/PPC64Ops.go | 2 +- src/cmd/compile/internal/ssa/gen/S390XOps.go | 2 +- src/cmd/compile/internal/ssa/opGen.go | 57 +++++++++++-------- test/nilptr3.go | 14 +++++ 9 files changed, 53 insertions(+), 32 deletions(-) diff --git a/src/cmd/compile/internal/ssa/gen/386Ops.go b/src/cmd/compile/internal/ssa/gen/386Ops.go index 9ec7c861a1..43388dfc22 100644 --- a/src/cmd/compile/internal/ssa/gen/386Ops.go +++ b/src/cmd/compile/internal/ssa/gen/386Ops.go @@ -439,7 +439,7 @@ func init() { // use of DX (the closure pointer) {name: "LoweredGetClosurePtr", reg: regInfo{outputs: []regMask{buildReg("DX")}}}, //arg0=ptr,arg1=mem, returns void. Faults if ptr is nil. - {name: "LoweredNilCheck", argLength: 2, reg: regInfo{inputs: []regMask{gpsp}}, clobberFlags: true, nilCheck: true}, + {name: "LoweredNilCheck", argLength: 2, reg: regInfo{inputs: []regMask{gpsp}}, clobberFlags: true, nilCheck: true, faultOnNilArg0: true}, // MOVLconvert converts between pointers and integers. // We have a special op for this so as to not confuse GC diff --git a/src/cmd/compile/internal/ssa/gen/AMD64Ops.go b/src/cmd/compile/internal/ssa/gen/AMD64Ops.go index 122512205d..f9739e90fc 100644 --- a/src/cmd/compile/internal/ssa/gen/AMD64Ops.go +++ b/src/cmd/compile/internal/ssa/gen/AMD64Ops.go @@ -486,7 +486,7 @@ func init() { // use of DX (the closure pointer) {name: "LoweredGetClosurePtr", reg: regInfo{outputs: []regMask{buildReg("DX")}}}, //arg0=ptr,arg1=mem, returns void. Faults if ptr is nil. - {name: "LoweredNilCheck", argLength: 2, reg: regInfo{inputs: []regMask{gpsp}}, clobberFlags: true, nilCheck: true}, + {name: "LoweredNilCheck", argLength: 2, reg: regInfo{inputs: []regMask{gpsp}}, clobberFlags: true, nilCheck: true, faultOnNilArg0: true}, // MOVQconvert converts between pointers and integers. // We have a special op for this so as to not confuse GC diff --git a/src/cmd/compile/internal/ssa/gen/ARM64Ops.go b/src/cmd/compile/internal/ssa/gen/ARM64Ops.go index 4002ab8abc..bbb175b0de 100644 --- a/src/cmd/compile/internal/ssa/gen/ARM64Ops.go +++ b/src/cmd/compile/internal/ssa/gen/ARM64Ops.go @@ -325,7 +325,7 @@ func init() { {name: "CALLinter", argLength: 2, reg: regInfo{inputs: []regMask{gp}, clobbers: callerSave}, aux: "Int64", clobberFlags: true, call: true}, // call fn by pointer. arg0=codeptr, arg1=mem, auxint=argsize, returns mem // pseudo-ops - {name: "LoweredNilCheck", argLength: 2, reg: regInfo{inputs: []regMask{gpg}}, nilCheck: true}, // panic if arg0 is nil. arg1=mem. + {name: "LoweredNilCheck", argLength: 2, reg: regInfo{inputs: []regMask{gpg}}, nilCheck: true, faultOnNilArg0: true}, // panic if arg0 is nil. arg1=mem. {name: "Equal", argLength: 1, reg: readflags}, // bool, true flags encode x==y false otherwise. {name: "NotEqual", argLength: 1, reg: readflags}, // bool, true flags encode x!=y false otherwise. diff --git a/src/cmd/compile/internal/ssa/gen/ARMOps.go b/src/cmd/compile/internal/ssa/gen/ARMOps.go index 313252ea88..a6db703d59 100644 --- a/src/cmd/compile/internal/ssa/gen/ARMOps.go +++ b/src/cmd/compile/internal/ssa/gen/ARMOps.go @@ -381,7 +381,7 @@ func init() { {name: "CALLinter", argLength: 2, reg: regInfo{inputs: []regMask{gp}, clobbers: callerSave}, aux: "Int64", clobberFlags: true, call: true}, // call fn by pointer. arg0=codeptr, arg1=mem, auxint=argsize, returns mem // pseudo-ops - {name: "LoweredNilCheck", argLength: 2, reg: regInfo{inputs: []regMask{gpg}}, nilCheck: true}, // panic if arg0 is nil. arg1=mem. + {name: "LoweredNilCheck", argLength: 2, reg: regInfo{inputs: []regMask{gpg}}, nilCheck: true, faultOnNilArg0: true}, // panic if arg0 is nil. arg1=mem. {name: "Equal", argLength: 1, reg: readflags}, // bool, true flags encode x==y false otherwise. {name: "NotEqual", argLength: 1, reg: readflags}, // bool, true flags encode x!=y false otherwise. diff --git a/src/cmd/compile/internal/ssa/gen/MIPS64Ops.go b/src/cmd/compile/internal/ssa/gen/MIPS64Ops.go index a103529296..537408779e 100644 --- a/src/cmd/compile/internal/ssa/gen/MIPS64Ops.go +++ b/src/cmd/compile/internal/ssa/gen/MIPS64Ops.go @@ -336,7 +336,7 @@ func init() { }, // pseudo-ops - {name: "LoweredNilCheck", argLength: 2, reg: regInfo{inputs: []regMask{gpg}}, nilCheck: true}, // panic if arg0 is nil. arg1=mem. + {name: "LoweredNilCheck", argLength: 2, reg: regInfo{inputs: []regMask{gpg}}, nilCheck: true, faultOnNilArg0: true}, // panic if arg0 is nil. arg1=mem. {name: "FPFlagTrue", argLength: 1, reg: readflags}, // bool, true if FP flag is true {name: "FPFlagFalse", argLength: 1, reg: readflags}, // bool, true if FP flag is false diff --git a/src/cmd/compile/internal/ssa/gen/PPC64Ops.go b/src/cmd/compile/internal/ssa/gen/PPC64Ops.go index 1fa84f6f25..d3f4703f89 100644 --- a/src/cmd/compile/internal/ssa/gen/PPC64Ops.go +++ b/src/cmd/compile/internal/ssa/gen/PPC64Ops.go @@ -288,7 +288,7 @@ func init() { {name: "LoweredGetClosurePtr", reg: regInfo{outputs: []regMask{ctxt}}}, //arg0=ptr,arg1=mem, returns void. Faults if ptr is nil. - {name: "LoweredNilCheck", argLength: 2, reg: regInfo{inputs: []regMask{gp | sp | sb}, clobbers: tmp}, clobberFlags: true, nilCheck: true}, + {name: "LoweredNilCheck", argLength: 2, reg: regInfo{inputs: []regMask{gp | sp | sb}, clobbers: tmp}, clobberFlags: true, nilCheck: true, faultOnNilArg0: true}, // Convert pointer to integer, takes a memory operand for ordering. {name: "MOVDconvert", argLength: 2, reg: gp11, asm: "MOVD"}, diff --git a/src/cmd/compile/internal/ssa/gen/S390XOps.go b/src/cmd/compile/internal/ssa/gen/S390XOps.go index 9c362ae5e6..3831de403a 100644 --- a/src/cmd/compile/internal/ssa/gen/S390XOps.go +++ b/src/cmd/compile/internal/ssa/gen/S390XOps.go @@ -370,7 +370,7 @@ func init() { // use of R12 (the closure pointer) {name: "LoweredGetClosurePtr", reg: regInfo{outputs: []regMask{buildReg("R12")}}}, // arg0=ptr,arg1=mem, returns void. Faults if ptr is nil. - {name: "LoweredNilCheck", argLength: 2, reg: regInfo{inputs: []regMask{ptrsp}}, clobberFlags: true, nilCheck: true}, + {name: "LoweredNilCheck", argLength: 2, reg: regInfo{inputs: []regMask{ptrsp}}, clobberFlags: true, nilCheck: true, faultOnNilArg0: true}, // MOVDconvert converts between pointers and integers. // We have a special op for this so as to not confuse GC diff --git a/src/cmd/compile/internal/ssa/opGen.go b/src/cmd/compile/internal/ssa/opGen.go index 4d47d2067c..8fae1685a9 100644 --- a/src/cmd/compile/internal/ssa/opGen.go +++ b/src/cmd/compile/internal/ssa/opGen.go @@ -4023,10 +4023,11 @@ var opcodeTable = [...]opInfo{ }, }, { - name: "LoweredNilCheck", - argLen: 2, - clobberFlags: true, - nilCheck: true, + name: "LoweredNilCheck", + argLen: 2, + clobberFlags: true, + nilCheck: true, + faultOnNilArg0: true, reg: regInfo{ inputs: []inputInfo{ {0, 255}, // AX CX DX BX SP BP SI DI @@ -6992,10 +6993,11 @@ var opcodeTable = [...]opInfo{ }, }, { - name: "LoweredNilCheck", - argLen: 2, - clobberFlags: true, - nilCheck: true, + name: "LoweredNilCheck", + argLen: 2, + clobberFlags: true, + nilCheck: true, + faultOnNilArg0: true, reg: regInfo{ inputs: []inputInfo{ {0, 65535}, // AX CX DX BX SP BP SI DI R8 R9 R10 R11 R12 R13 R14 R15 @@ -10081,9 +10083,10 @@ var opcodeTable = [...]opInfo{ }, }, { - name: "LoweredNilCheck", - argLen: 2, - nilCheck: true, + name: "LoweredNilCheck", + argLen: 2, + nilCheck: true, + faultOnNilArg0: true, reg: regInfo{ inputs: []inputInfo{ {0, 6143}, // R0 R1 R2 R3 R4 R5 R6 R7 R8 R9 g R12 @@ -12200,9 +12203,10 @@ var opcodeTable = [...]opInfo{ }, }, { - name: "LoweredNilCheck", - argLen: 2, - nilCheck: true, + name: "LoweredNilCheck", + argLen: 2, + nilCheck: true, + faultOnNilArg0: true, reg: regInfo{ inputs: []inputInfo{ {0, 268173311}, // R0 R1 R2 R3 R4 R5 R6 R7 R8 R9 R10 R11 R12 R13 R14 R15 R16 R17 R19 R20 R21 R22 R23 R24 R25 R26 g @@ -13817,9 +13821,10 @@ var opcodeTable = [...]opInfo{ }, }, { - name: "LoweredNilCheck", - argLen: 2, - nilCheck: true, + name: "LoweredNilCheck", + argLen: 2, + nilCheck: true, + faultOnNilArg0: true, reg: regInfo{ inputs: []inputInfo{ {0, 100663294}, // R1 R2 R3 R4 R5 R6 R7 R8 R9 R10 R11 R12 R13 R14 R15 R16 R17 R18 R19 R20 R21 R22 R24 R25 g @@ -15195,10 +15200,11 @@ var opcodeTable = [...]opInfo{ }, }, { - name: "LoweredNilCheck", - argLen: 2, - clobberFlags: true, - nilCheck: true, + name: "LoweredNilCheck", + argLen: 2, + clobberFlags: true, + nilCheck: true, + faultOnNilArg0: true, reg: regInfo{ inputs: []inputInfo{ {0, 1073733630}, // SP SB R3 R4 R5 R6 R7 R8 R9 R10 R11 R12 R14 R15 R16 R17 R18 R19 R20 R21 R22 R23 R24 R25 R26 R27 R28 R29 @@ -17511,10 +17517,11 @@ var opcodeTable = [...]opInfo{ }, }, { - name: "LoweredNilCheck", - argLen: 2, - clobberFlags: true, - nilCheck: true, + name: "LoweredNilCheck", + argLen: 2, + clobberFlags: true, + nilCheck: true, + faultOnNilArg0: true, reg: regInfo{ inputs: []inputInfo{ {0, 37886}, // R1 R2 R3 R4 R5 R6 R7 R8 R9 R12 SP diff --git a/test/nilptr3.go b/test/nilptr3.go index a97873b97c..b965cd262d 100644 --- a/test/nilptr3.go +++ b/test/nilptr3.go @@ -233,3 +233,17 @@ func c1() { var x Struct func() { x.m() }() // ERROR "removed nil check" } + +type SS struct { + x byte +} + +type TT struct { + SS +} + +func f(t *TT) *byte { + // See issue 17242. + s := &t.SS // ERROR "removed nil check" + return &s.x // ERROR "generated nil check" +}