-
Notifications
You must be signed in to change notification settings - Fork 13k
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
1.26.0 cannot optimize length of vector collected from a slice of known size #50925
Comments
Can you try on nightly? |
Nightly doesn't elide the allocation either: playground::main:
pushq %rbx
subq $64, %rsp
movq $8, 8(%rsp)
pxor %xmm0, %xmm0
movdqu %xmm0, 16(%rsp)
movl $24, %edi
movl $8, %esi
callq __rust_alloc@PLT
testq %rax, %rax
je .LBB10_5
movq %rax, 8(%rsp)
movq $3, 16(%rsp)
xorl %ecx, %ecx
testb %cl, %cl
jne .LBB10_3
leaq .Lbyte_str.a+4(%rip), %rcx
movq %rcx, %xmm0
leaq .Lbyte_str.a(%rip), %rcx
movq %rcx, %xmm1
punpcklqdq %xmm0, %xmm1
movdqu %xmm1, (%rax)
leaq .Lbyte_str.a+8(%rip), %rcx
movq %rcx, 16(%rax)
movl $3, %ecx
.LBB10_3:
movq %rcx, 24(%rsp)
movdqu 8(%rsp), %xmm0
movdqa %xmm0, 32(%rsp)
movq %rcx, 48(%rsp)
cmpq $3, 48(%rsp)
jne .LBB10_4
movq 40(%rsp), %rsi
testq %rsi, %rsi
je .LBB10_9
movq 32(%rsp), %rdi
shlq $3, %rsi
movl $8, %edx
callq __rust_dealloc@PLT
.LBB10_9:
addq $64, %rsp
popq %rbx
retq
.LBB10_5:
callq alloc::alloc::oom@PLT
jmp .LBB10_6
.LBB10_4:
callq std::panicking::begin_panic
.LBB10_6:
ud2
movq %rax, %rbx
leaq 32(%rsp), %rdi
callq core::ptr::drop_in_place
movq %rbx, %rdi
callq _Unwind_Resume@PLT
ud2
movq %rax, %rbx
leaq 8(%rsp), %rdi
callq core::ptr::drop_in_place
movq %rbx, %rdi
callq _Unwind_Resume@PLT
ud2 |
If you compile with panic=abort it looks like the regression goes away. Sure enough looking at the IR I see: ; invoke alloc::alloc::oom
invoke void @_ZN5alloc5alloc3oom17h12c4e4476d085e6dE()
to label %.noexc10.i.i.i unwind label %bb1.i.i.i, !noalias !11 If I change that to I think we may need to add |
Ironically, we want to move oom to be allowed to unwind... #50880 (comment) |
I'm marking this as T-libs since it seems like a "libs policy" decision as much as anything, no? |
I don't really understand why oom unwinding would prevent LLVM from eliding allocations. It seems like we should be able to teach it to do the right thing here, right? |
@sfackler in general I believe LLVM has pattern recognition to realize when code allocates, doesn't use the data, and then deallocates. In that situation LLVM will entirely eliminate the allocation. In the example above the allocation is never actually used, so LLVM should be able to optimize it out. With the invoke instruction, however, the destructor looks like it may touch the data (as the function call isn't inlined) so the allocation isn't inlined away. With the |
OOM can't unwind today, and historically it's been optimized as if it can't unwind. This accidentally regressed with recent changes to the OOM handler, so this commit adds in a codegen test to assert that everything gets optimized away after the OOM function is approrpiately classified as nounwind Closes rust-lang#50925
std: Ensure OOM is classified as `nounwind` OOM can't unwind today, and historically it's been optimized as if it can't unwind. This accidentally regressed with recent changes to the OOM handler, so this commit adds in a codegen test to assert that everything gets optimized away after the OOM function is approrpiately classified as nounwind Closes #50925
The code:
With rustc 1.25.0:
With rustc 1.26.0:
The text was updated successfully, but these errors were encountered: