cmd/compile: x86-64: sub-optimal codegen less-than-int sized index from structured slice/array

[bupjae]

Go version

go version go1.25-devel_6c3b5a2 Thu Jul 3 18:43:56 2025 -0700 linux/amd64

Output of go env in your module/workspace:

Using service on https://go.godbolt.org/
Selected "x86-64 gc (tip)"

What did you do?

I entered following program

package main

func Foo(a [][4]byte, b []int, i int) int {
	return b[a[i][1]]
}

func main() {
}

https://go.godbolt.org/z/hfvezofxf

What did you see happen?

Codegen for a[i][1] part was following:

        LEAQ    (AX)(R9*4), DX
        LEAQ    1(DX), DX
        MOVBLZX (DX), AX

What did you expect to see?

One-line codegen like MOVBLZX 1(AX)(R9*4), AX was expected

The compiler really worked very hard to optimize codegen regarding various element size of slice a. However, it seems that it missed simple case - when the element size is 2, 4 or 8.

[randall77]

I think this is just a case of missing strides. We have only implemented strides of 1 and the load size so far. It is a lot of extra opcodes and rewrite rules :(
In this case, it is a load size of 1 but a stride of 4.

There are two LEAQs instead of one because of #21735. Presumably we could fold the constant offset into the load itself to save one of those LEAQs.

[gabyhelp]

Related Issues

• cmd/compile: improve code generation for temporary slice copy and inlining #18529
• cmd/compile: optimize slice access with constant upper/lower bound #14266 (closed)
• cmd/compile: remove bounds checking for sub-slices #14905 (closed)
• cmd/compile: unexpected difference in compiled code for returned array #31198
• cmd/internal/gc: accessing an array with power of two 2 size should use left shifts not multiplications #10638 (closed)
• cmd/compile: assembly generated is bigger than previous versions #30229 (closed)
• cmd/compile: unsafe conversion from slice to struct pointer generates worse code on amd64 than on 386 #65330
• cmd/compile: unnecessary array copying in for range loop #33838 (closed)

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

[Jorropo]

Aside of what @randall77 already said, should this snippet of code not use INCQ DX since the source and destination match ?

[bupjae]

Then, correct way would be adding SSA opcodes such as MOVBloadidx4 and rewrite rules.

Looks like this issue is marked as TODO long time ago

go/src/cmd/compile/internal/ssa/_gen/AMD64Ops.go

Lines 841 to 865 in 86b5ba7

	// MOVxloadidx: indexed loads
	// load (Q=8,L=4,W=2,B=1) bytes from (arg0+scale*arg1+auxint+aux), arg2=mem.
	// Results are zero-extended. (TODO: sign-extending indexed loads)
	{name: "MOVBloadidx1", argLength: 3, reg: gploadidx, commutative: true, asm: "MOVBLZX", scale: 1, aux: "SymOff", typ: "UInt8", symEffect: "Read"},
	{name: "MOVWloadidx1", argLength: 3, reg: gploadidx, commutative: true, asm: "MOVWLZX", scale: 1, aux: "SymOff", typ: "UInt16", symEffect: "Read"},
	{name: "MOVWloadidx2", argLength: 3, reg: gploadidx, asm: "MOVWLZX", scale: 2, aux: "SymOff", typ: "UInt16", symEffect: "Read"},
	{name: "MOVLloadidx1", argLength: 3, reg: gploadidx, commutative: true, asm: "MOVL", scale: 1, aux: "SymOff", typ: "UInt32", symEffect: "Read"},
	{name: "MOVLloadidx4", argLength: 3, reg: gploadidx, asm: "MOVL", scale: 4, aux: "SymOff", typ: "UInt32", symEffect: "Read"},
	{name: "MOVLloadidx8", argLength: 3, reg: gploadidx, asm: "MOVL", scale: 8, aux: "SymOff", typ: "UInt32", symEffect: "Read"},
	{name: "MOVQloadidx1", argLength: 3, reg: gploadidx, commutative: true, asm: "MOVQ", scale: 1, aux: "SymOff", typ: "UInt64", symEffect: "Read"},
	{name: "MOVQloadidx8", argLength: 3, reg: gploadidx, asm: "MOVQ", scale: 8, aux: "SymOff", typ: "UInt64", symEffect: "Read"},
	// MOVxstoreidx: indexed stores
	// Store (Q=8,L=4,W=2,B=1) low bytes of arg2.
	// Does *(arg0+scale*arg1+auxint+aux) = arg2, arg3=mem.
	{name: "MOVBstoreidx1", argLength: 4, reg: gpstoreidx, commutative: true, asm: "MOVB", scale: 1, aux: "SymOff", symEffect: "Write"},
	{name: "MOVWstoreidx1", argLength: 4, reg: gpstoreidx, commutative: true, asm: "MOVW", scale: 1, aux: "SymOff", symEffect: "Write"},
	{name: "MOVWstoreidx2", argLength: 4, reg: gpstoreidx, asm: "MOVW", scale: 2, aux: "SymOff", symEffect: "Write"},
	{name: "MOVLstoreidx1", argLength: 4, reg: gpstoreidx, commutative: true, asm: "MOVL", scale: 1, aux: "SymOff", symEffect: "Write"},
	{name: "MOVLstoreidx4", argLength: 4, reg: gpstoreidx, asm: "MOVL", scale: 4, aux: "SymOff", symEffect: "Write"},
	{name: "MOVLstoreidx8", argLength: 4, reg: gpstoreidx, asm: "MOVL", scale: 8, aux: "SymOff", symEffect: "Write"},
	{name: "MOVQstoreidx1", argLength: 4, reg: gpstoreidx, commutative: true, asm: "MOVQ", scale: 1, aux: "SymOff", symEffect: "Write"},
	{name: "MOVQstoreidx8", argLength: 4, reg: gpstoreidx, asm: "MOVQ", scale: 8, aux: "SymOff", symEffect: "Write"},
	// TODO: add size-mismatched indexed loads/stores, like MOVBstoreidx4?

Related work - adding SSA opcode: #40426 . Although it seems that how to write rewrite rule has been changed significantly since then.

[gopherbot]

Change https://go.dev/cl/687976 mentions this issue: cmd/internal/pkgsite: increase x/pkgsite dependency to CL 687918

[randall77]

Note that now that we have the addressingmodes pass, the combinatorial explosion, particularly in the rules files, won't be as bad.