cmd/gc: don't generate checknils when slicing an array

cmd/gc: don't generate checknils when slicing an array

Compiling

func f() int {
        buf := make([]byte, 1024)
        return len(buf)
}

now generates

"".f t=1 size=80 value=0 args=0x8 locals=0x418
        0x0000 00000 (x.go:3)	TEXT	"".f+0(SB),$1048-8
        0x0000 00000 (x.go:3)	MOVQ	(TLS),CX
        0x0009 00009 (x.go:3)	LEAQ	-920(SP),AX
        0x0011 00017 (x.go:3)	CMPQ	AX,(CX)
        0x0014 00020 (x.go:3)	JHI	,29
        0x0016 00022 (x.go:3)	CALL	,runtime.morestack8_noctxt(SB)
        0x001b 00027 (x.go:3)	JMP	,0
        0x001d 00029 (x.go:3)	SUBQ	$1048,SP
        0x0024 00036 (x.go:3)	FUNCDATA	$2,gclocals·a7a3692b8e27e823add69ec4239ba55f+0(SB)
        0x0024 00036 (x.go:3)	FUNCDATA	$3,gclocals·0528ab8f76149a707fd2f0025c2178a3+0(SB)
        0x0024 00036 (x.go:4)	LEAQ	"".autotmp_0000+0(SP),DI
        0x0028 00040 (x.go:4)	MOVL	$0,AX
        0x002a 00042 (x.go:4)	DUFFZERO	,$
        0x002f 00047 (x.go:4)	LEAQ	"".autotmp_0000+0(SP),BX
        0x0033 00051 (x.go:4)	MOVQ	$1024,AX
        0x003a 00058 (x.go:5)	MOVQ	AX,"".~r0+1056(FP)
        0x0042 00066 (x.go:5)	ADDQ	$1048,SP
        0x0049 00073 (x.go:5)	RET	,

This is not quite perfect, but it is good enough for now.

Fixes issue 7316.

[josharian, 2014-04-24 01:36:34]

Hello rsc@golang.org (cc: golang-codereviews@googlegroups.com),

I'd like you to review this change to
https://code.google.com/p/go

[josharian, 2014-04-24 01:37:39]

I didn't see an obvious way to accomplish this without adding a field to Node.
Tips/pointers most welcome.

[josharian, 2014-05-05 18:18:05]

The savings from this CL are small and the release is coming up soon. I'll close
this and mail again for Go 1.4.

FWIW, I just came across another case in which a Node notnil flag might be
useful--walk.c:733 (walkexpr's OAS2MAPR case):

// TODO: ptr is always non-nil, so disable nil check for this OIND op.

[gobot, 2014-05-05 18:18:26]

R=close (assigned by josharian@gmail.com)

[rsc, 2014-05-09 19:50:48]

FWIW, this is too precise a fix. There is a general problem here not being
addressed.

[josharian, 2014-05-09 20:21:57]

> FWIW, this is too precise a fix. There is a general problem here not being
> addressed.

Yep, I realized that shortly after closing the CL. There's also naming confusion
in the CL between slices, pointers to arrays, and arrays. (Eep.)

I'll try for a deeper and broader fix after 1.3. If you already have opinions
about the general shape of that fix, I'd love to hear them.

[rsc, 2014-05-09 20:24:36]

The broader fix I had in mind is any slice of an array doesn't need the nil
check.

[josharian, 2014-05-10 00:29:44]

> The broader fix I had in mind is any slice of an array doesn't need the nil
> check.

Doing that naively causes this case from test/nilptr.go to fail:

func p3() {
	// Array to slice.
	var p *[1 << 30]byte = nil
	var x []byte = p[0:] // should panic
	_ = x
}

An even broader fix would be that any generated pointer into the stack doesn't
need a nil check, but I'm not sure how to accomplish that.

Anyway, I'll think about this more after 1.3 is out.

[rsc, 2014-06-17 16:27:29]

On 2014/05/10 00:29:44, josharian wrote:
> > The broader fix I had in mind is any slice of an array doesn't need the nil
> > check.
> 
> Doing that naively causes this case from test/nilptr.go to fail:

That's not a slice of an array. That's a slice of an array pointer.

[josharian, 2014-08-01 00:21:40]

> The broader fix I had in mind is any slice of an array doesn't need the nil
check.

Done.

This prevents 142 checknils while compiling the stdlib. Is it worth it?


I failed in my attempts not to introduce the nonnil flag. The obvious thing to
do is to have cgen_slice support slicing arrays directly, rather than pointers
to arrays. I took a stab at that in CL 121000044 but it felt awkward and I got
stuck on broken nested arrays/slices (all.bash fails).

I'll try again if you'd prefer that route and think it is worth pursuing.


> I just came across another case in which a Node notnil flag might be
useful--walkexpr's OAS2MAPR case

I implemented this in CL 114630043 (not for review yet, just for reference).
Again, I don't know whether it is worth the code. Are there further cases in
which we could omit a checknil but don't?

[rsc, 2014-08-25 00:34:10]

i think we can leave this for later. it's not important enough and it's not
obviously correct.