cmd/compile: opt: eliminate redundant cap check

[adonovan]

While investigating a crash in gopls, I noticed that the assembly for this function contains what appears to be a redundant check of n <= cap(r.data):

type reader struct{ data []byte }

func (r *reader) bytes(n int) []byte {
	v := r.data[:n]
	r.data = r.data[n:]
	return v
}

Why is the cap check needed? It is immediately followed by a similar check against r.data.len, which should (it seems to me) suffice. Perhaps we could optimize it out.

main.(*reader).bytes STEXT size=176 args=0x10 locals=0x18 funcid=0x0 align=0x0
	0x0000 00000 	L0008	TEXT	main.(*reader).bytes(SB), ABIInternal, $32-16
	0x0000 00000 	L0008	MOVD	16(g), R16
	0x0004 00004 	L0008	PCDATA	$0, $-2
	0x0004 00004 	L0008	CMP	R16, RSP
	0x0008 00008 	L0008	BLS	148

	0x000c 00012 	L0008	PCDATA	$0, $-1
	0x000c 00012 	L0008	MOVD.W	R30, -32(RSP)
	0x0010 00016 	L0008	MOVD	R29, -8(RSP)
	0x0014 00020 	L0008	SUB	$8, RSP, R29
	0x0018 00024 	L0008	FUNCDATA	$0, gclocals·2NSbawKySWs0upw55xaGlw==(SB)
	0x0018 00024 	L0008	FUNCDATA	$1, gclocals·ISb46fRPFoZ9pIfykFK/kQ==(SB)
	0x0018 00024 	L0008	FUNCDATA	$5, main.(*reader).bytes.arginfo1(SB)
	0x0018 00024 	L0008	FUNCDATA	$6, main.(*reader).bytes.argliveinfo(SB)
	0x0018 00024 	L0008	PCDATA	$3, $1
	0x0018 00024 	L0009	MOVD	16(R0), R2	; r2 = r.data.cap
	0x001c 00028 	L0009	CMP	R2, R1		; n < r.data.cap 
	0x0020 00032 	L0009	BHI	140		; if n >= r.data.cap { panicSliceAcap() } 

	0x0024 00036 	L0009	MOVD	8(R0), R3	; r3 = r.data.len
	0x0028 00040 	L0010	CMP	R3, R1		; n < r.data.len 
	0x002c 00044 	L0010	BHI	128		; if n >= r.data.len { panicSliceB() } 

	0x0030 00048 	L0009	MOVD	(R0), R4
	0x0034 00052 	L0010	SUB	R1, R2, R5
	0x0038 00056 	L0010	SUB	R1, R3, R3
	0x003c 00060 	L0010	MOVD	R3, 8(R0)
	0x0040 00064 	L0010	MOVD	R5, 16(R0)
	0x0044 00068 	L0010	NEG	R5, R3
	0x0048 00072 	L0010	AND	R3->63, R1, R3
	0x004c 00076 	L0010	ADD	R3, R4, R3
	0x0050 00080 	L0010	PCDATA	$0, $-3
	0x0050 00080 	L0010	MOVWU	runtime.writeBarrier(SB), R5
	0x0058 00088 	L0010	PCDATA	$0, $-1
	0x0058 00088 	L0010	PCDATA	$0, $-2
	0x0058 00088 	L0010	CBZW	R5, 108
	0x005c 00092 	L0010	CALL	runtime.gcWriteBarrier2(SB)
	0x0060 00096 	L0010	MOVD	R3, (R25)
	0x0064 00100 	L0010	MOVD	(R0), R5
	0x0068 00104 	L0010	MOVD	R5, 8(R25)
	0x006c 00108 	L0010	MOVD	R3, (R0)
	0x0070 00112 	L0011	PCDATA	$0, $-1
	0x0070 00112 	L0011	MOVD	R4, R0
	0x0074 00116 	L0011	MOVD	-8(RSP), R29
	0x0078 00120 	L0011	MOVD.P	32(RSP), R30
	0x007c 00124 	L0011	RET	(R30)

	0x0080 00128 	L0010	MOVD	R1, R0
	0x0084 00132 	L0010	MOVD	R3, R1
	0x0088 00136 	L0010	PCDATA	$1, $1
	0x0088 00136 	L0010	CALL	runtime.panicSliceB(SB)

	0x008c 00140 	L0009	CALL	runtime.panicSliceAcap(SB)
	0x0090 00144 	L0009	HINT	$0

	0x0094 00148 	L0009	NOP
	0x0094 00148 	L0008	PCDATA	$1, $-1
	0x0094 00148 	L0008	PCDATA	$0, $-2
	0x0094 00148 	L0008	STP	(R0, R1), 8(RSP)
	0x0098 00152 	L0008	MOVD	R30, R3
	0x009c 00156 	L0008	CALL	runtime.morestack_noctxt(SB)
	0x00a0 00160 	L0008	PCDATA	$0, $-1
	0x00a0 00160 	L0008	LDP	8(RSP), (R0, R1)
	0x00a4 00164 	L0008	JMP	0

[prattmic]

cc @golang/compiler

[gabyhelp]

Related Issues

• cmd/compile: eliminate more bounds checks #17370
• cmd/compile: eliminate some bounds checks #5364 (closed)
• cmd/compile: elide call to runtime.growslice with provable capacity availability #30509
• cmd/compile: missing BCE on slice in loop #15203 (closed)
• cmd/compile: elide write barriers for pointer swaps #16765 (closed)
• cmd/compile: checking len(data) and erroring out if too small does not help BCE #19126 (closed)
• cmd/compile: bounds check not removed after "if index < length" #15074 (closed)
• cmd/compile: unneccesary 0 check in division #45928 (closed)
• cmd/compile: apparently extraneous inlmarks #31618 (closed)

Related Code Changes

• cmd/compile: optimize len check when make slice

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

[randall77]

If n is bigger than the capacity, we want a different panic than if n is between the length and capacity.
The former panics at r.data[:n], the latter at r.data[n:]. In particular, if n is between the length and capacity the first one doesn't panic and the second one does.

package main

import "fmt"

func main() {
	x := make([]int, 4, 8)
	f1(x)
	f2(x)
	f3(x)
	f4(x)
}

func f1(x []int) {
	defer func() {
		fmt.Printf("f1 %s\n", recover())
	}()
	_ = x[:6]
}
func f2(x []int) {
	defer func() {
		fmt.Printf("f2 %s\n", recover())
	}()
	_ = x[6:]
}
func f3(x []int) {
	defer func() {
		fmt.Printf("f3 %s\n", recover())
	}()
	_ = x[:9]
}
func f4(x []int) {
	defer func() {
		fmt.Printf("f4 %s\n", recover())
	}()
	_ = x[9:]
}

f1 %!s(<nil>)
f2 runtime error: slice bounds out of range [6:4]
f3 runtime error: slice bounds out of range [:9] with capacity 8
f4 runtime error: slice bounds out of range [9:4]

The real question is why this has a write barrier.
I thought we handled slice updates (r.data = r.data[n:]) without them.

[adonovan]

Ah, of course, the first slice operation isn't an error. Sorry for the dumb question, and thanks for the detailed explanation.

The real question is why this has a write barrier.

Hah, I didn't even notice that.

[randall77]

The real question is why this has a write barrier.
I thought we handled slice updates (r.data = r.data[n:]) without them.

I think I was thinking of r.data = append(r.data, x), which correctly does not do the pointer write when we don't need to grow the backing store.

But still, we should do r.data = r.data[n:] without write barriers. I will open a separate issue.