about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2022-05-02 09:05:22 +0000
committerbors <bors@rust-lang.org>2022-05-02 09:05:22 +0000
commit6b6c1ffacc5df738b3560369746d87499adbeae1 (patch)
tree5e27b6bf9e8c16318305cd0b2fc05e44ca89e8ec
parent905fd731543ee837bd9f4cf960b048610d037155 (diff)
parent2830dbd64f50b4a8025025f01578e45cbf9d3719 (diff)
downloadrust-6b6c1ffacc5df738b3560369746d87499adbeae1.tar.gz
rust-6b6c1ffacc5df738b3560369746d87499adbeae1.zip
Auto merge of #96596 - scottmcm:limited-calloc, r=Mark-Simulacrum
Tweak the vec-calloc runtime check to only apply to shortish-arrays

r? `@Mark-Simulacrum`

`@nbdd0121` pointed out in https://github.com/rust-lang/rust/pull/95362#issuecomment-1114085395 that LLVM currently doesn't constant-fold the `IsZero` check for long arrays, so that seems like a reasonable justification for limiting it.

It appears that it's based on length, not byte size, (https://godbolt.org/z/4s48Y81dP), so that's what I used in the PR.  Maybe it's a ["the number of inlining shall be three"](https://youtu.be/s4wnuiCwTGU?t=320) sort of situation.

Certainly there's more that could be done here -- that generated code that checks long arrays byte-by-byte is highly suboptimal, for example -- but this is an easy, low-risk tweak.
-rw-r--r--library/alloc/src/vec/is_zero.rs9
-rw-r--r--src/test/codegen/vec-calloc.rs32
2 files changed, 40 insertions, 1 deletions
diff --git a/library/alloc/src/vec/is_zero.rs b/library/alloc/src/vec/is_zero.rs
index 868f2f1e323..edf270db81d 100644
--- a/library/alloc/src/vec/is_zero.rs
+++ b/library/alloc/src/vec/is_zero.rs
@@ -52,7 +52,14 @@ unsafe impl<T> IsZero for *mut T {
 unsafe impl<T: IsZero, const N: usize> IsZero for [T; N] {
     #[inline]
     fn is_zero(&self) -> bool {
-        self.iter().all(IsZero::is_zero)
+        // Because this is generated as a runtime check, it's not obvious that
+        // it's worth doing if the array is really long.  The threshold here
+        // is largely arbitrary, but was picked because as of 2022-05-01 LLVM
+        // can const-fold the check in `vec![[0; 32]; n]` but not in
+        // `vec![[0; 64]; n]`: https://godbolt.org/z/WTzjzfs5b
+        // Feel free to tweak if you have better evidence.
+
+        N <= 32 && self.iter().all(IsZero::is_zero)
     }
 }
 
diff --git a/src/test/codegen/vec-calloc.rs b/src/test/codegen/vec-calloc.rs
new file mode 100644
index 00000000000..c616e9f1145
--- /dev/null
+++ b/src/test/codegen/vec-calloc.rs
@@ -0,0 +1,32 @@
+// compile-flags: -O
+// only-x86_64
+// ignore-debug
+
+#![crate_type = "lib"]
+
+// CHECK-LABEL: @vec_zero_scalar
+#[no_mangle]
+pub fn vec_zero_scalar(n: usize) -> Vec<i32> {
+    // CHECK-NOT: __rust_alloc(
+    // CHECK: __rust_alloc_zeroed(
+    // CHECK-NOT: __rust_alloc(
+    vec![0; n]
+}
+
+// CHECK-LABEL: @vec_zero_rgb48
+#[no_mangle]
+pub fn vec_zero_rgb48(n: usize) -> Vec<[u16; 3]> {
+    // CHECK-NOT: __rust_alloc(
+    // CHECK: __rust_alloc_zeroed(
+    // CHECK-NOT: __rust_alloc(
+    vec![[0, 0, 0]; n]
+}
+
+// CHECK-LABEL: @vec_zero_array_32
+#[no_mangle]
+pub fn vec_zero_array_32(n: usize) -> Vec<[i64; 32]> {
+    // CHECK-NOT: __rust_alloc(
+    // CHECK: __rust_alloc_zeroed(
+    // CHECK-NOT: __rust_alloc(
+    vec![[0_i64; 32]; n]
+}