Skip to content

Commit

Permalink
Auto merge of rust-lang#130998 - the8472:bail-before-memcpy, r=<try>
Browse files Browse the repository at this point in the history
avoid phi node for pointers flowing into Vec appends

related discussion: https://rust-lang.zulipchat.com/#narrow/stream/187780-t-compiler.2Fwg-llvm/topic/nocapture.20and.20allocation.20elimination

r? ghost
  • Loading branch information
bors committed Sep 30, 2024
2 parents fb4aebd + c44e0f4 commit fd59d69
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 0 deletions.
6 changes: 6 additions & 0 deletions library/alloc/src/slice.rs
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,12 @@ pub(crate) mod hack {
impl<T: Copy> ConvertVec for T {
#[inline]
fn to_vec<A: Allocator>(s: &[Self], alloc: A) -> Vec<Self, A> {
if s.is_empty() {
// The early return is not necessary for correctness, but it helps
// LLVM by avoiding phi nodes flowing into memcpy.
// See codegen/lib-optimizations/append-elements.rs
return Vec::new_in(alloc);
}
let mut v = Vec::with_capacity_in(s.len(), alloc);
// SAFETY:
// allocated above with the capacity of `s`, and initialize to `s.len()` in
Expand Down
6 changes: 6 additions & 0 deletions library/alloc/src/vec/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2519,6 +2519,12 @@ impl<T, A: Allocator> Vec<T, A> {
#[inline]
unsafe fn append_elements(&mut self, other: *const [T]) {
let count = unsafe { (*other).len() };
if count == 0 {
// The early return is not necessary for correctness, but it helps
// LLVM by avoiding phi nodes flowing into memcpy.
// See codegen/lib-optimizations/append-elements.rs
return;
}
self.reserve(count);
let len = self.len();
unsafe { ptr::copy_nonoverlapping(other as *const T, self.as_mut_ptr().add(len), count) };
Expand Down
37 changes: 37 additions & 0 deletions tests/codegen/lib-optimizations/append-elements.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
//@ compile-flags: -O
#![crate_type = "lib"]

//! Check that pointer flowing into vec.append comes straight from an allocation
//! and not through a phi node that merges the allocation and zero-length cases.
//! With this and https://github.com/llvm/llvm-project/pull/110280 the intermediate
//! allocation should be optimized away in the future.

// CHECK-LABEL: @vec_append_with_temp_alloc
#[no_mangle]
pub fn vec_append_with_temp_alloc(dst: &mut Vec<u8>, src: &[u8]) {
// CHECK: %[[TEMP:.+]] = tail call noalias noundef ptr @__rust_alloc

// First memcpy, it uses the src pointer directly
// CHECK: call void @llvm.memcpy.{{.*}}%src.0
let temp = src.to_vec();

// final memcpy to destination
// CHECK: call void @llvm.memcpy.{{.*}}%dst.i{{.*}}%[[TEMP]]
dst.extend(&temp);
// CHECK: ret
}

// CHECK-LABEL: @string_append_with_temp_alloc
#[no_mangle]
pub fn string_append_with_temp_alloc(dst: &mut String, src: &str) {
// CHECK: %[[TEMP:.+]] = tail call noalias noundef ptr @__rust_alloc

// First memcpy, it uses the src pointer directly
// CHECK: call void @llvm.memcpy.{{.*}}%src.0
let temp = src.to_string();

// final memcpy to destination
// CHECK: call void @llvm.memcpy.{{.*}}%dst.i{{.*}}%[[TEMP]]
dst.push_str(&temp);
// CHECK: ret
}

0 comments on commit fd59d69

Please sign in to comment.