Add separate libcalls for array.copy w/ vs. w/out GC ref elems#13312
Add separate libcalls for array.copy w/ vs. w/out GC ref elems#13312fitzgen wants to merge 1 commit intobytecodealliance:mainfrom
Conversation
Addresses a long-standing `TODO` comment whose performance implications were also observed in bytecodealliance#13279
alexcrichton
left a comment
There was a problem hiding this comment.
Could you add a test as well that takes ~little time with this PR but maybe ~many seconds without this PR? Out of curiosity, how much does this help #13279?
Also, as a separate question, would it make sense to delete one or both of these libcalls eventually? For example with GC arrays I figure it'd be best to just do the loop in Cranelift, and for non-GC arrays I figure it might be best to have a single memcpy libcall that's pretty raw and that way we could funnel memory.copy through there too. That way we could const-propagate a lot of things here like the checked_mul and array offsets and such which might be a noticable boost.
| // SAFETY: Both pointers are derived from the same `VMGcObjectData` | ||
| // and are within the bounds we already checked. `core::ptr::copy` | ||
| // handles the overlapping case correctly, and no GC can occur | ||
| // because we are only copying non-GC-ref elements. | ||
| unsafe { | ||
| let src_ptr = data.slice(src_byte_start, byte_len).as_ptr(); | ||
| let dst_ptr = data.slice_mut(dst_byte_start, byte_len).as_mut_ptr(); | ||
| core::ptr::copy(src_ptr, dst_ptr, usize::try_from(byte_len).unwrap()); | ||
| } |
There was a problem hiding this comment.
Were we to run this through miri I think it would flag this as a violation as data.slice_mut I believe invalidates the previous src_ptr derived. Would it be possible to use copy_within here?
Addresses a long-standing
TODOcomment whose performance implications were also observed in #13279(Still investigating the panic that issue's benchmarks trigger in the copying collector, however.)