[tankf33der]

Important note: At first I wanted to add tests in this commit, but then decided against it. I'll make a separate PR because I still need to touch GitHub CI too.

Happy reading.

This is an important first PR from a future series that will fix closure on all broken architectures.
Future PRs must and will reference it.

From the very first day when I dove into the wonderful world of assembly and closure implementations on different architectures, it was clear that they all differ. And one half is correct, while the other half is incorrect.
If you take a quick glance at all architectures, you'll easily see the key distinguishing feature - the use of 1 floating point register.

It remains to find or create a test that will show there is a problem. This turned out to be not so simple because you need to consider 4 (!) points simultaneously, where each combination is unique - architectures + implementation + compiler + registers.

Current riscv64 implementation:

thunk:
        auipc t6, 0xffffc
	ld    t5, 8(t6)
	jr    t5
GET DATA:
        ld    a0, 0(t6)
        ret

The author waits and hopes that between calls of two subroutines, the value in register t6 that is important for operation will be waiting for him.

This is assembler for call __CLOSURE_GET_DATA from gcc:

        .globl  anon_fn_ecd785f68341be0a_int_int_int_int_int_int_int_int_int_int_int_int_int_int_int__int_60849
        .hidden anon_fn_ecd785f68341be0a_int_int_int_int_int_int_int_int_int_int_int_int_int_int_int__int_60849
        .type   anon_fn_ecd785f68341be0a_int_int_int_int_int_int_int_int_int_int_int_int_int_int_int__int_60849, @function
anon_fn_ecd785f68341be0a_int_int_int_int_int_int_int_int_int_int_int_int_int_int_int__int_60849:
.LFB197:
        .cfi_startproc
        addi    sp,sp,-64
        .cfi_def_cfa_offset 64
        sd      ra,56(sp)
        sd      s0,48(sp)
        .cfi_offset 1, -8
        .cfi_offset 8, -16
        addi    s0,sp,64
        .cfi_def_cfa 8, 0
        mv      t4,a0               <----- ALWAYS only
        mv      t3,a1               <----- three
        mv      t1,a2               <----- temp registers
        mv      a0,a3
        mv      a1,a4
        mv      a2,a5
        mv      a3,a6
        mv      a4,a7
        mv      a5,t4
        sw      a5,-36(s0)
        mv      a5,t3
        sw      a5,-40(s0)
        mv      a5,t1
        sw      a5,-44(s0)
        mv      a5,a0
        sw      a5,-48(s0)
        mv      a5,a1
        sw      a5,-52(s0)
        mv      a5,a2
        sw      a5,-56(s0)
        mv      a5,a3
        sw      a5,-60(s0)
        mv      a5,a4
        sw      a5,-64(s0)
        lla     a5,__CLOSURE_GET_DATA
        ld      a5,0(a5)
        jalr    a5
        sd      a0,-24(s0)
        li      a5,-123
        mv      a0,a5
        ld      ra,56(sp)
        .cfi_restore 1
        ld      s0,48(sp)
        .cfi_restore 8
        .cfi_def_cfa 2, 64
        addi    sp,sp,64
        .cfi_def_cfa_offset 0
        jr      ra
        .cfi_endproc

The author saw that gcc doesn't use other registers and self-confidently uses t6.

clang reuses out temp register (clang 17-19):

        .hidden anon_fn_ecd785f68341be0a_int_int_int_int_int_int_int_int_int_int_int_int_int_int_int__int_60849 # -- Begin function anon_fn_ecd785f68341be0a_int_int_int_int_int_int_int_int_
        .globl  anon_fn_ecd785f68341be0a_int_int_int_int_int_int_int_int_int_int_int_int_int_int_int__int_60849
        .p2align        1
        .type   anon_fn_ecd785f68341be0a_int_int_int_int_int_int_int_int_int_int_int_int_int_int_int__int_60849,@function
anon_fn_ecd785f68341be0a_int_int_int_int_int_int_int_int_int_int_int_int_int_int_int__int_60849: # @anon_fn_ecd785f68341be0a_int_int_int_int_int_int_int_int_int_int_int_int_int_int_int__int
        .cfi_startproc
# %bb.0:
        addi    sp, sp, -96
        .cfi_def_cfa_offset 96
        sd      ra, 88(sp)                      # 8-byte Folded Spill
        sd      s0, 80(sp)                      # 8-byte Folded Spill
        sd      s1, 72(sp)                      # 8-byte Folded Spill
        .cfi_offset ra, -8
        .cfi_offset s0, -16
        .cfi_offset s1, -24
        addi    s0, sp, 96
        .cfi_def_cfa s0, 0
        mv      t0, a6
        mv      t1, a5
        mv      t2, a4
        mv      t3, a3
        mv      t4, a2
        mv      t5, a1
        mv      t6, a0                    <----------- BOOM!
        ld      a0, 48(s0)
        ld      a1, 40(s0)
        ld      a2, 32(s0)
        ld      a3, 24(s0)
        ld      a4, 16(s0)
        ld      a5, 8(s0)
        ld      a6, 0(s0)
                                        # kill: def $x9 killed $x17
                                        # kill: def $x9 killed $x5
                                        # kill: def $x9 killed $x6
                                        # kill: def $x9 killed $x7
                                        # kill: def $x9 killed $x28
                                        # kill: def $x9 killed $x29
                                        # kill: def $x9 killed $x30
                                        # kill: def $x9 killed $x31
        sw      t6, -28(s0)
        sw      t5, -32(s0)
        sw      t4, -36(s0)
        sw      t3, -40(s0)
        sw      t2, -44(s0)
        sw      t1, -48(s0)
        sw      t0, -52(s0)
        sw      a7, -56(s0)
        sw      a6, -60(s0)
        sw      a5, -64(s0)
        sw      a4, -68(s0)
        sw      a3, -72(s0)
        sw      a2, -76(s0)
        sw      a1, -80(s0)
        sw      a0, -84(s0)
.Lpcrel_hi203:
        auipc   a0, %pcrel_hi(__CLOSURE_GET_DATA)
        addi    a0, a0, %pcrel_lo(.Lpcrel_hi203)
        ld      a0, 0(a0)
        jalr    a0
        sd      a0, -96(s0)
        li      a0, -123
        ld      ra, 88(sp)                      # 8-byte Folded Reload
        ld      s0, 80(sp)                      # 8-byte Folded Reload
        ld      s1, 72(sp)                      # 8-byte Folded Reload
        addi    sp, sp, 96
        ret

Now it's clear why this stub is placed in the important test v/tests/fns/closure_generator_test.v to prevent crashing on clang. :) BTW arm64 closures are seems broken too.

fn get_max_string_params() int {
        $if macos || (clang && arm64) {
                return 9
        }
        return 16
}

The new implementation simply uses a floating point temporary register instead of a general-purpose temporary register, as in other architectures. There's nowhere else to store it, and the stack frame is not available to us.

GitHub CI won't break for now, I'm not touching tests yet too.

v/tests - OK with gcc and clang.
all closures - OK with gcc and clang.

[huly-for-github]

Connected to Huly®: V_0.6-22691

[spytheman]

Excellent work.