cmd/compile: avoid call to runtime.makeslice for zero-length types?

[bradfitz]

Go version

Go 1.22

What did you do?

return make([]struct{}, 123) compiles to a call to runtime.makeslice:

        LEAQ    type:struct {}(SB), AX
        MOVL    $123, BX
        MOVQ    BX, CX
        PCDATA  $1, $0
        NOP
        CALL    runtime.makeslice(SB)
        MOVL    $123, BX
        MOVQ    BX, CX
        ADDQ    $24, SP
        POPQ    BP
        RET

We see this show up in profiles (small, but non-zero) for calls to https://pkg.go.dev/tailscale.com/types/views#Slice.LenIter (which predates Go 1.22's range-over-int):

// LenIter returns a slice the same length as the v.Len().
// The caller can then range over it to get the valid indexes.
// It does not allocate.
func (v Slice[T]) LenIter() []struct{} { return make([]struct{}, len(v.ж)) }

I realize that the compiler could omit those makeslice calls when the width of the type being made is zero (as there's no actual allocation).

The compiler already does that when the slice length is zero:

// Recognise make([]T, 0) and replace it with a pointer to the zerobase
(StaticLECall {callAux} _ (Const(64|32) [0]) (Const(64|32) [0]) mem)
        && isSameCall(callAux, "runtime.makeslice")
        => (MakeResult (Addr <v.Type.FieldType(0)> {ir.Syms.Zerobase} (SB)) mem)

/cc @golang/compiler @twitchyliquid64 @maisem

[Jorropo]

I added this exact optimization for zero sized slices, I didn't added it for zero sized types since I couldn't figure out how to recover the compiler *types.Type for a compiled type pointer in the SSA passes and this wasn't what I was trying to optimize so I never investigated it.

After 5 minutes of searching I did back then we loose the type information of the compiled type pointer after genssa and talking with the frontend object isn't trivial.

[Jorropo]

We could do this optimization in genssa as we have all the required type info but it's tricky because we can't rely on prove to properly prove that the indexes are correctly in bounds. Because this needs to panic:

var a = -1
_ = make([]struct{}, a)

During genssa we can check if the arguments are literals or constants but anything else is very hard fast.

for i := range make([]struct{}, 10) { // this would be optimized
 for j := range make([]struct{}, i) { /* ... */ } // here optimizing it in genssa would fail while SSA can use prove to handle this
}

Going back to your example:

func (v Slice[T]) LenIter() []struct{} { return make([]struct{}, len(v.ж)) }

This is wouldn't be optimized in genssa as before prove we don't know what len(v.ж) could be. Actually that fine, len can't return an invalid length (*incorrect unsafe code aside, but it's fine if improper uses of unsafe leads to bug)
I don't even know if prove would even properly handle this because it fails on some edge cases with len and makeslice.

---

Imo this should be handled in a third party linter which finds all the ranges of make([]zeroSizedType, n) calls and ask pretty please update the code to use range over int, git grep -F "make([]struct{}, " should get you 95% of the way there already.

[mauri870]

Regarding the SSA rule, I have come up with this:

(StaticLECall {callAux} (Addr {t} _) _ _ mem)
	&& isSameCall(callAux, "runtime.makeslice")
	&& t.(*obj.LSym).String() == "type:struct {}"
	=> (MakeResult (Addr <v.Type.FieldType(0)> {ir.Syms.Zerobase} (SB)) mem)

But I have to admit that I lack proper understanding of this, there are probably better ways to achieve the same, if it is even possible this early in the optimization passes.

[Jorropo]

@mauri870 this doesn't panic when the length and capacity are out of bounds like it should.

---

Checking the string lsym is an interesting idea but it's very flashy.
Wouldn't work with something like:

type empty struct{}

// ...
_ = make([]empty, n)

altho that not that bad.

[mauri870]

I agree, my approach is very naive. There are probably lots of problems, I'm happy if it serves as a starting point somehow. Thanks for looking into this.

[Jorropo]

I will make a patch which implements this in internal/walk which would handle literals, constants and len / cap calls as arguments to make if the compiler team think this a suitable (cc @randall77 ).
But imo consumers should rewrite their code to use range over int instead.

[randall77]

Yes, that's fine.
We could do it for all make calls, generating the negative test on the make argument and calling runtime.panicmakeslicelen if so. Not sure if it is worth it.

[Jorropo]

I thought we should inline hot code paths not cold ones.