Skip to content

Array bound tests with for loop that get removed with while loops #81253

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
leonardo-m opened this issue Jan 21, 2021 · 3 comments
Open

Array bound tests with for loop that get removed with while loops #81253

leonardo-m opened this issue Jan 21, 2021 · 3 comments
Labels
A-mir-opt Area: MIR optimizations C-enhancement Category: An issue proposing an enhancement or a PR with one. C-optimization Category: An issue highlighting optimization opportunities or PRs implementing such I-slow Issue: Problems and improvements with respect to performance of generated code.

Comments

@leonardo-m
Copy link

I think there's something wrong with Rust loops, they could lose some range information. So even if a for loop should be slightly simpler to analize compared to a while loop, the foo2 has no array bound tests, unlike foo1 (if N becomes 10 both get compiled to about the same asm without array bound tests):

pub fn foo1() -> u32 {
    const N: usize = 20;
    let mut data = [[0; N + 2]; N + 2];
    for x in 0 .. N + 2 {
        for y in 0 .. N + 2 {
            if x == 1 || x == N || y == 1 || y == N {
                data[x][y] = 1;
            }
        }
    }
    data[0][0]
}

pub fn foo2() -> u32 {
    const N: usize = 20;
    let mut data = [[0; N + 2]; N + 2];
    let mut x = 0;
    while x < N + 2 {
        let mut y = 0;
        while y < N + 2 {
            if x == 1 || x == N || y == 1 || y == N {
                data[x][y] = 1;
            }
            y += 1;
        }
        x += 1;
    }
    data[0][0]
}

rustc (V.1.51.0-nightly a4cbb44 2021-01-20) gives:

foo1:
    push    rbx
    sub     rsp, 1936
    mov     rdi, rsp
    xor     ebx, ebx
    mov     edx, 1936
    xor     esi, esi
    call    qword ptr [rip + memset@GOTPCREL]
    mov     al, 1
    vbroadcastss    ymm0, dword ptr [rip + .LCPI0_0]
    xor     edi, edi
    jmp     .LBB0_1
.LBB0_9:
    vmovups ymmword ptr [rsp + rbx + 88], ymm0
    vmovups ymmword ptr [rsp + rbx + 120], ymm0
    vmovups xmmword ptr [rsp + rbx + 152], xmm0
    mov     ecx, 21
    mov     edx, 20
.LBB0_10:
    add     rdi, 2
    lea     rdx, [rsp + 4*rdx]
    mov     dword ptr [rbx + rdx + 88], 1
    lea     rcx, [rsp + 4*rcx]
    mov     dword ptr [rbx + rcx + 88], 1
    cmp     rax, 21
    setb    al
    add     rbx, 176
    cmp     rbx, 1936
    je      .LBB0_11
.LBB0_1:
    cmp     rbx, 1760
    jne     .LBB0_6
    test    al, 1
    je      .LBB0_8
    vmovups ymmword ptr [rsp + rbx], ymm0
    vmovups ymmword ptr [rsp + rbx + 32], ymm0
    vmovups xmmword ptr [rsp + rbx + 64], xmm0
    mov     ecx, 21
    mov     edx, 20
    jmp     .LBB0_4
.LBB0_6:
    test    al, 1
    je      .LBB0_8
    mov     ecx, 20
    mov     edx, 1
.LBB0_4:
    lea     rax, [rdi + 1]
    lea     rdx, [rsp + 4*rdx]
    mov     dword ptr [rbx + rdx], 1
    lea     rcx, [rsp + 4*rcx]
    mov     dword ptr [rbx + rcx], 1
    test    rbx, rbx
    je      .LBB0_9
    mov     ecx, 20
    mov     edx, 1
    jmp     .LBB0_10
.LBB0_11:
    mov     eax, dword ptr [rsp]
    add     rsp, 1936
    pop     rbx
    vzeroupper
    ret
.LBB0_8:
    lea     rdx, [rip + .L__unnamed_1]
    mov     esi, 22
    vzeroupper
    call    qword ptr [rip + core::panicking::panic_bounds_check@GOTPCREL]
    ud2


foo2:
    push    rbx
    sub     rsp, 1936
    mov     rdi, rsp
    xor     ebx, ebx
    mov     edx, 1936
    xor     esi, esi
    call    qword ptr [rip + memset@GOTPCREL]
    vbroadcastss    ymm0, dword ptr [rip + .LCPI1_0]
    jmp     .LBB1_1
.LBB1_3:
    lea     rsi, [rsp + 4*rsi]
    mov     dword ptr [rbx + rsi], 1
    lea     rdx, [rsp + 4*rdx]
    mov     dword ptr [rbx + rdx], 1
    test    rbx, rbx
    je      .LBB1_4
.LBB1_5:
    lea     rcx, [rsp + 4*rcx]
    mov     dword ptr [rbx + rcx + 88], 1
    lea     rax, [rsp + 4*rax]
    mov     dword ptr [rbx + rax + 88], 1
    add     rbx, 176
    cmp     rbx, 1936
    je      .LBB1_6
.LBB1_1:
    mov     eax, 20
    mov     ecx, 1
    mov     esi, 1
    mov     edx, 20
    cmp     rbx, 1760
    jne     .LBB1_3
    vmovups ymmword ptr [rsp + rbx], ymm0
    vmovups ymmword ptr [rsp + rbx + 32], ymm0
    vmovups xmmword ptr [rsp + rbx + 64], xmm0
    mov     edx, 21
    mov     esi, 20
    jmp     .LBB1_3
.LBB1_4:
    vmovups ymmword ptr [rsp + rbx + 88], ymm0
    vmovups ymmword ptr [rsp + rbx + 120], ymm0
    vmovups xmmword ptr [rsp + rbx + 152], xmm0
    mov     eax, 21
    mov     ecx, 20
    jmp     .LBB1_5
.LBB1_6:
    mov     eax, dword ptr [rsp]
    add     rsp, 1936
    pop     rbx
    vzeroupper
    ret
@camelid
Copy link
Member

camelid commented Jan 25, 2021

Just to clarify, the issue is that an optimization is only applied in some cases? This doesn't have the potential to causes bugs, right?

@camelid
Copy link
Member

camelid commented Jan 25, 2021

So even if a for loop should be slightly simpler to analize compared to a while loop,

Actually I think while loops are potentially simpler to analyze since for loops are just sugar for iterators.

@leonardo-m
Copy link
Author

the issue is that an optimization is only applied in some cases? This doesn't have the potential to causes bugs, right?

Right, that's why I didn't tag this issue as Bug. It's an enhancement request, in practice. But it may show some troubles in how Rustc handles loops.

Actually I think while loops are potentially simpler to analyze since for loops are just sugar for iterators.

I see, you're right, thank you.

@camelid camelid added C-enhancement Category: An issue proposing an enhancement or a PR with one. I-slow Issue: Problems and improvements with respect to performance of generated code. A-mir-opt Area: MIR optimizations labels Jan 29, 2021
@workingjubilee workingjubilee added the C-optimization Category: An issue highlighting optimization opportunities or PRs implementing such label Oct 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-mir-opt Area: MIR optimizations C-enhancement Category: An issue proposing an enhancement or a PR with one. C-optimization Category: An issue highlighting optimization opportunities or PRs implementing such I-slow Issue: Problems and improvements with respect to performance of generated code.
Projects
None yet
Development

No branches or pull requests

3 participants