cmd/compile: re-enable nilcheck removal for newobject

Also add compiler debug ouput and add a test.

Fixes #15390.

Change-Id: Iceba1414c29bcc213b87837387bf8ded1f3157f1
Reviewed-on: https://go-review.googlesource.com/30011
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>

src/cmd/compile/internal/ssa/config.go

@@ -17,6 +17,7 @@ type Config struct {
 	arch            string                     // "amd64", etc.
 	IntSize         int64                      // 4 or 8
 	PtrSize         int64                      // 4 or 8
+	RegSize         int64                      // 4 or 8
 	lowerBlock      func(*Block, *Config) bool // lowering function
 	lowerValue      func(*Value, *Config) bool // lowering function
 	registers       []Register                 // machine registers
@@ -132,6 +133,7 @@ func NewConfig(arch string, fe Frontend, ctxt *obj.Link, optimize bool) *Config
 	case "amd64":
 		c.IntSize = 8
 		c.PtrSize = 8
+		c.RegSize = 8
 		c.lowerBlock = rewriteBlockAMD64
 		c.lowerValue = rewriteValueAMD64
 		c.registers = registersAMD64[:]
@@ -142,6 +144,7 @@ func NewConfig(arch string, fe Frontend, ctxt *obj.Link, optimize bool) *Config
 	case "amd64p32":
 		c.IntSize = 4
 		c.PtrSize = 4
+		c.RegSize = 8
 		c.lowerBlock = rewriteBlockAMD64
 		c.lowerValue = rewriteValueAMD64
 		c.registers = registersAMD64[:]
@@ -153,6 +156,7 @@ func NewConfig(arch string, fe Frontend, ctxt *obj.Link, optimize bool) *Config
 	case "386":
 		c.IntSize = 4
 		c.PtrSize = 4
+		c.RegSize = 4
 		c.lowerBlock = rewriteBlock386
 		c.lowerValue = rewriteValue386
 		c.registers = registers386[:]
@@ -163,6 +167,7 @@ func NewConfig(arch string, fe Frontend, ctxt *obj.Link, optimize bool) *Config
 	case "arm":
 		c.IntSize = 4
 		c.PtrSize = 4
+		c.RegSize = 4
 		c.lowerBlock = rewriteBlockARM
 		c.lowerValue = rewriteValueARM
 		c.registers = registersARM[:]
@@ -173,6 +178,7 @@ func NewConfig(arch string, fe Frontend, ctxt *obj.Link, optimize bool) *Config
 	case "arm64":
 		c.IntSize = 8
 		c.PtrSize = 8
+		c.RegSize = 8
 		c.lowerBlock = rewriteBlockARM64
 		c.lowerValue = rewriteValueARM64
 		c.registers = registersARM64[:]
@@ -187,6 +193,7 @@ func NewConfig(arch string, fe Frontend, ctxt *obj.Link, optimize bool) *Config
 	case "ppc64le":
 		c.IntSize = 8
 		c.PtrSize = 8
+		c.RegSize = 8
 		c.lowerBlock = rewriteBlockPPC64
 		c.lowerValue = rewriteValuePPC64
 		c.registers = registersPPC64[:]
@@ -199,6 +206,7 @@ func NewConfig(arch string, fe Frontend, ctxt *obj.Link, optimize bool) *Config
 	case "mips64", "mips64le":
 		c.IntSize = 8
 		c.PtrSize = 8
+		c.RegSize = 8
 		c.lowerBlock = rewriteBlockMIPS64
 		c.lowerValue = rewriteValueMIPS64
 		c.registers = registersMIPS64[:]
@@ -210,6 +218,7 @@ func NewConfig(arch string, fe Frontend, ctxt *obj.Link, optimize bool) *Config
 	case "s390x":
 		c.IntSize = 8
 		c.PtrSize = 8
+		c.RegSize = 8
 		c.lowerBlock = rewriteBlockS390X
 		c.lowerValue = rewriteValueS390X
 		c.registers = registersS390X[:]

src/cmd/compile/internal/ssa/gen/generic.rules

@@ -958,13 +958,15 @@
 	-> mem
 // nil checks just need to rewrite to something useless.
 // they will be deadcode eliminated soon afterwards.
-        //(NilCheck (Load (OffPtr [c] (SP)) mem) mem)
-        //        && mem.Op == OpStaticCall
-        //	&& isSameSym(mem.Aux, "runtime.newobject")
-        //	&& c == config.ctxt.FixedFrameSize() + config.PtrSize // offset of return value
-        //	-> (Invalid)
-        //(NilCheck (OffPtr (Load (OffPtr [c] (SP)) mem)) mem)
-        //        && mem.Op == OpStaticCall
-        //	&& isSameSym(mem.Aux, "runtime.newobject")
-        //	&& c == config.ctxt.FixedFrameSize() + config.PtrSize // offset of return value
-        //	-> (Invalid)
+(NilCheck (Load (OffPtr [c] (SP)) mem) mem)
+	&& mem.Op == OpStaticCall
+	&& isSameSym(mem.Aux, "runtime.newobject")
+	&& c == config.ctxt.FixedFrameSize() + config.RegSize // offset of return value
+	&& warnRule(config.Debug_checknil() && int(v.Line) > 1, v, "removed nil check")
+	-> (Invalid)
+(NilCheck (OffPtr (Load (OffPtr [c] (SP)) mem)) mem)
+	&& mem.Op == OpStaticCall
+	&& isSameSym(mem.Aux, "runtime.newobject")
+	&& c == config.ctxt.FixedFrameSize() + config.RegSize // offset of return value
+	&& warnRule(config.Debug_checknil() && int(v.Line) > 1, v, "removed nil check")
+	-> (Invalid)

src/cmd/compile/internal/ssa/rewrite.go

@@ -370,6 +370,15 @@ func noteRule(s string) bool {
 	return true
 }
 
+// warnRule generates a compiler debug output with string s when
+// cond is true and the rule is fired.
+func warnRule(cond bool, v *Value, s string) bool {
+	if cond {
+		v.Block.Func.Config.Warnl(v.Line, "removed nil check")
+	}
+	return true
+}
+
 // logRule logs the use of the rule s. This will only be enabled if
 // rewrite rules were generated with the -log option, see gen/rulegen.go.
 func logRule(s string) {

src/cmd/compile/internal/ssa/rewritegeneric.go

@@ -6380,6 +6380,64 @@ func rewriteValuegeneric_OpNilCheck(v *Value, config *Config) bool {
 		v.AddArg(mem)
 		return true
 	}
+	// match: (NilCheck (Load (OffPtr [c] (SP)) mem) mem)
+	// cond: mem.Op == OpStaticCall 	&& isSameSym(mem.Aux, "runtime.newobject") 	&& c == config.ctxt.FixedFrameSize() + config.RegSize 	&& warnRule(config.Debug_checknil() && int(v.Line) > 1, v, "removed nil check")
+	// result: (Invalid)
+	for {
+		v_0 := v.Args[0]
+		if v_0.Op != OpLoad {
+			break
+		}
+		v_0_0 := v_0.Args[0]
+		if v_0_0.Op != OpOffPtr {
+			break
+		}
+		c := v_0_0.AuxInt
+		v_0_0_0 := v_0_0.Args[0]
+		if v_0_0_0.Op != OpSP {
+			break
+		}
+		mem := v_0.Args[1]
+		if mem != v.Args[1] {
+			break
+		}
+		if !(mem.Op == OpStaticCall && isSameSym(mem.Aux, "runtime.newobject") && c == config.ctxt.FixedFrameSize()+config.RegSize && warnRule(config.Debug_checknil() && int(v.Line) > 1, v, "removed nil check")) {
+			break
+		}
+		v.reset(OpInvalid)
+		return true
+	}
+	// match: (NilCheck (OffPtr (Load (OffPtr [c] (SP)) mem)) mem)
+	// cond: mem.Op == OpStaticCall 	&& isSameSym(mem.Aux, "runtime.newobject") 	&& c == config.ctxt.FixedFrameSize() + config.RegSize 	&& warnRule(config.Debug_checknil() && int(v.Line) > 1, v, "removed nil check")
+	// result: (Invalid)
+	for {
+		v_0 := v.Args[0]
+		if v_0.Op != OpOffPtr {
+			break
+		}
+		v_0_0 := v_0.Args[0]
+		if v_0_0.Op != OpLoad {
+			break
+		}
+		v_0_0_0 := v_0_0.Args[0]
+		if v_0_0_0.Op != OpOffPtr {
+			break
+		}
+		c := v_0_0_0.AuxInt
+		v_0_0_0_0 := v_0_0_0.Args[0]
+		if v_0_0_0_0.Op != OpSP {
+			break
+		}
+		mem := v_0_0.Args[1]
+		if mem != v.Args[1] {
+			break
+		}
+		if !(mem.Op == OpStaticCall && isSameSym(mem.Aux, "runtime.newobject") && c == config.ctxt.FixedFrameSize()+config.RegSize && warnRule(config.Debug_checknil() && int(v.Line) > 1, v, "removed nil check")) {
+			break
+		}
+		v.reset(OpInvalid)
+		return true
+	}
 	return false
 }
 func rewriteValuegeneric_OpNot(v *Value, config *Config) bool {

test/nilptr3.go

@@ -247,3 +247,10 @@ func f(t *TT) *byte {
 	s := &t.SS  // ERROR "removed nil check"
 	return &s.x // ERROR "generated nil check"
 }
+
+// make sure not to do nil check for newobject
+func f7() (*Struct, float64) {
+	t := new(Struct)
+	p := &t.Y    // ERROR "removed nil check"
+	return t, *p // ERROR "removed nil check"
+}