forked from rust-lang/rust
-
Notifications
You must be signed in to change notification settings - Fork 0
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
FCP WIP #1
Comments
Submitted at rust-lang#116226 (comment). |
ChaiTRex
pushed a commit
that referenced
this issue
Oct 28, 2024
…raheemdev Optimize `Box::default` and `Arc::default` to construct more types in place Both the `Arc` and `Box` `Default` impls currently call `T::default()` before allocating, and then moving the resulting `T` into the allocation. Most `Default` impls are trivial, which should in theory allow LLVM to construct `T: Default` directly in the `Box` allocation when calling `<Box<T>>::default()`. However, the allocation may fail, which necessitates calling `T`'s destructor if it has one. If the destructor is non-trivial, then LLVM has a hard time proving that it's sound to elide, which makes it construct `T` on the stack first, and then copy it into the allocation. Change both of these impls to allocate first, and then call `T::default` into the uninitialized allocation, so that LLVM doesn't have to prove that it's sound to elide the destructor/initial stack copy. For example, given the following Rust code: ```rust #[derive(Default, Clone)] struct Foo { x: Vec<u8>, z: String, y: Vec<u8>, } #[no_mangle] pub fn src() -> Box<Foo> { Box::default() } ``` <details open> <summary>Before this PR:</summary> ```llvm `@__rust_no_alloc_shim_is_unstable` = external global i8 ; drop_in_place() generated in case the allocation fails ; core::ptr::drop_in_place<playground::Foo> ; Function Attrs: nounwind nonlazybind uwtable define internal fastcc void `@"_ZN4core3ptr36drop_in_place$LT$playground..Foo$GT$17hff376aece491233bE"(ptr` noalias nocapture noundef readonly align 8 dereferenceable(72) %_1) unnamed_addr #0 personality ptr `@rust_eh_personality` { start: %_1.val = load i64, ptr %_1, align 8 %0 = icmp eq i64 %_1.val, 0 br i1 %0, label %bb6, label %"_ZN63_$LT$alloc..alloc..Global$u20$as$u20$core..alloc..Allocator$GT$10deallocate17heaa87468709346b1E.exit.i.i.i4.i" "_ZN63_$LT$alloc..alloc..Global$u20$as$u20$core..alloc..Allocator$GT$10deallocate17heaa87468709346b1E.exit.i.i.i4.i": ; preds = %start %1 = getelementptr inbounds i8, ptr %_1, i64 8 %_1.val6 = load ptr, ptr %1, align 8, !nonnull !3, !noundef !3 tail call void `@__rust_dealloc(ptr` noundef nonnull %_1.val6, i64 noundef %_1.val, i64 noundef 1) rust-lang#8 br label %bb6 bb6: ; preds = %"_ZN63_$LT$alloc..alloc..Global$u20$as$u20$core..alloc..Allocator$GT$10deallocate17heaa87468709346b1E.exit.i.i.i4.i", %start %2 = getelementptr inbounds i8, ptr %_1, i64 24 %.val9 = load i64, ptr %2, align 8 %3 = icmp eq i64 %.val9, 0 br i1 %3, label %bb5, label %"_ZN63_$LT$alloc..alloc..Global$u20$as$u20$core..alloc..Allocator$GT$10deallocate17heaa87468709346b1E.exit.i.i.i4.i.i11" "_ZN63_$LT$alloc..alloc..Global$u20$as$u20$core..alloc..Allocator$GT$10deallocate17heaa87468709346b1E.exit.i.i.i4.i.i11": ; preds = %bb6 %4 = getelementptr inbounds i8, ptr %_1, i64 32 %.val10 = load ptr, ptr %4, align 8, !nonnull !3, !noundef !3 tail call void `@__rust_dealloc(ptr` noundef nonnull %.val10, i64 noundef %.val9, i64 noundef 1) rust-lang#8 br label %bb5 bb5: ; preds = %"_ZN63_$LT$alloc..alloc..Global$u20$as$u20$core..alloc..Allocator$GT$10deallocate17heaa87468709346b1E.exit.i.i.i4.i.i11", %bb6 %5 = getelementptr inbounds i8, ptr %_1, i64 48 %.val4 = load i64, ptr %5, align 8 %6 = icmp eq i64 %.val4, 0 br i1 %6, label %"_ZN4core3ptr46drop_in_place$LT$alloc..vec..Vec$LT$u8$GT$$GT$17hb5ca95423e113cf7E.exit16", label %"_ZN63_$LT$alloc..alloc..Global$u20$as$u20$core..alloc..Allocator$GT$10deallocate17heaa87468709346b1E.exit.i.i.i4.i15" "_ZN63_$LT$alloc..alloc..Global$u20$as$u20$core..alloc..Allocator$GT$10deallocate17heaa87468709346b1E.exit.i.i.i4.i15": ; preds = %bb5 %7 = getelementptr inbounds i8, ptr %_1, i64 56 %.val5 = load ptr, ptr %7, align 8, !nonnull !3, !noundef !3 tail call void `@__rust_dealloc(ptr` noundef nonnull %.val5, i64 noundef %.val4, i64 noundef 1) rust-lang#8 br label %"_ZN4core3ptr46drop_in_place$LT$alloc..vec..Vec$LT$u8$GT$$GT$17hb5ca95423e113cf7E.exit16" "_ZN4core3ptr46drop_in_place$LT$alloc..vec..Vec$LT$u8$GT$$GT$17hb5ca95423e113cf7E.exit16": ; preds = %bb5, %"_ZN63_$LT$alloc..alloc..Global$u20$as$u20$core..alloc..Allocator$GT$10deallocate17heaa87468709346b1E.exit.i.i.i4.i15" ret void } ; Function Attrs: nonlazybind uwtable define noalias noundef nonnull align 8 ptr `@src()` unnamed_addr #1 personality ptr `@rust_eh_personality` { start: ; alloca to place `Foo` in. %_1 = alloca [72 x i8], align 8 call void `@llvm.lifetime.start.p0(i64` 72, ptr nonnull %_1) store i64 0, ptr %_1, align 8 %_2.sroa.4.0._1.sroa_idx = getelementptr inbounds i8, ptr %_1, i64 8 store ptr inttoptr (i64 1 to ptr), ptr %_2.sroa.4.0._1.sroa_idx, align 8 %_2.sroa.5.0._1.sroa_idx = getelementptr inbounds i8, ptr %_1, i64 16 %_3.sroa.4.0..sroa_idx = getelementptr inbounds i8, ptr %_1, i64 32 call void `@llvm.memset.p0.i64(ptr` noundef nonnull align 8 dereferenceable(16) %_2.sroa.5.0._1.sroa_idx, i8 0, i64 16, i1 false) store ptr inttoptr (i64 1 to ptr), ptr %_3.sroa.4.0..sroa_idx, align 8 %_3.sroa.5.0..sroa_idx = getelementptr inbounds i8, ptr %_1, i64 40 %_4.sroa.4.0..sroa_idx = getelementptr inbounds i8, ptr %_1, i64 56 call void `@llvm.memset.p0.i64(ptr` noundef nonnull align 8 dereferenceable(16) %_3.sroa.5.0..sroa_idx, i8 0, i64 16, i1 false) store ptr inttoptr (i64 1 to ptr), ptr %_4.sroa.4.0..sroa_idx, align 8 %_4.sroa.5.0..sroa_idx = getelementptr inbounds i8, ptr %_1, i64 64 store i64 0, ptr %_4.sroa.5.0..sroa_idx, align 8 %0 = load volatile i8, ptr `@__rust_no_alloc_shim_is_unstable,` align 1, !noalias !4 %_0.i.i.i = tail call noalias noundef align 8 dereferenceable_or_null(72) ptr `@__rust_alloc(i64` noundef 72, i64 noundef 8) rust-lang#8, !noalias !4 %1 = icmp eq ptr %_0.i.i.i, null br i1 %1, label %bb2.i, label %"_ZN5alloc5boxed12Box$LT$T$GT$3new17h0864de14f863a27aE.exit" bb2.i: ; preds = %start ; invoke alloc::alloc::handle_alloc_error invoke void `@_ZN5alloc5alloc18handle_alloc_error17h98142d0d8d74161bE(i64` noundef 8, i64 noundef 72) rust-lang#9 to label %.noexc unwind label %cleanup.i .noexc: ; preds = %bb2.i unreachable cleanup.i: ; preds = %bb2.i %2 = landingpad { ptr, i32 } cleanup ; call core::ptr::drop_in_place<playground::Foo> call fastcc void `@"_ZN4core3ptr36drop_in_place$LT$playground..Foo$GT$17hff376aece491233bE"(ptr` noalias noundef nonnull align 8 dereferenceable(72) %_1) rust-lang#10 resume { ptr, i32 } %2 "_ZN5alloc5boxed12Box$LT$T$GT$3new17h0864de14f863a27aE.exit": ; preds = %start ; Copy from stack to heap if allocation is successful call void `@llvm.memcpy.p0.p0.i64(ptr` noundef nonnull align 8 dereferenceable(72) %_0.i.i.i, ptr noundef nonnull align 8 dereferenceable(72) %_1, i64 72, i1 false) call void `@llvm.lifetime.end.p0(i64` 72, ptr nonnull %_1) ret ptr %_0.i.i.i } ``` </details> <details> <summary>After this PR</summary> ```llvm ; Notice how there's no `drop_in_place()` generated as well define noalias noundef nonnull align 8 ptr `@src()` unnamed_addr #0 personality ptr `@rust_eh_personality` { start: ; no stack allocation %0 = load volatile i8, ptr `@__rust_no_alloc_shim_is_unstable,` align 1 %_0.i.i.i.i.i = tail call noalias noundef align 8 dereferenceable_or_null(72) ptr `@__rust_alloc(i64` noundef 72, i64 noundef 8) rust-lang#5 %1 = icmp eq ptr %_0.i.i.i.i.i, null br i1 %1, label %bb3.i, label %"_ZN5alloc5boxed16Box$LT$T$C$A$GT$13new_uninit_in17h80d6355ef4b73ea3E.exit" bb3.i: ; preds = %start ; call alloc::alloc::handle_alloc_error tail call void `@_ZN5alloc5alloc18handle_alloc_error17h98142d0d8d74161bE(i64` noundef 8, i64 noundef 72) rust-lang#6 unreachable "_ZN5alloc5boxed16Box$LT$T$C$A$GT$13new_uninit_in17h80d6355ef4b73ea3E.exit": ; preds = %start ; construct `Foo` directly into the allocation if successful store i64 0, ptr %_0.i.i.i.i.i, align 8 %_8.sroa.4.0._1.sroa_idx = getelementptr inbounds i8, ptr %_0.i.i.i.i.i, i64 8 store ptr inttoptr (i64 1 to ptr), ptr %_8.sroa.4.0._1.sroa_idx, align 8 %_8.sroa.5.0._1.sroa_idx = getelementptr inbounds i8, ptr %_0.i.i.i.i.i, i64 16 %_8.sroa.7.0._1.sroa_idx = getelementptr inbounds i8, ptr %_0.i.i.i.i.i, i64 32 tail call void `@llvm.memset.p0.i64(ptr` noundef nonnull align 8 dereferenceable(16) %_8.sroa.5.0._1.sroa_idx, i8 0, i64 16, i1 false) store ptr inttoptr (i64 1 to ptr), ptr %_8.sroa.7.0._1.sroa_idx, align 8 %_8.sroa.8.0._1.sroa_idx = getelementptr inbounds i8, ptr %_0.i.i.i.i.i, i64 40 %_8.sroa.10.0._1.sroa_idx = getelementptr inbounds i8, ptr %_0.i.i.i.i.i, i64 56 tail call void `@llvm.memset.p0.i64(ptr` noundef nonnull align 8 dereferenceable(16) %_8.sroa.8.0._1.sroa_idx, i8 0, i64 16, i1 false) store ptr inttoptr (i64 1 to ptr), ptr %_8.sroa.10.0._1.sroa_idx, align 8 %_8.sroa.11.0._1.sroa_idx = getelementptr inbounds i8, ptr %_0.i.i.i.i.i, i64 64 store i64 0, ptr %_8.sroa.11.0._1.sroa_idx, align 8 ret ptr %_0.i.i.i.i.i } ``` </details>
ChaiTRex
pushed a commit
that referenced
this issue
Oct 28, 2024
…Kobzol shave 150ms off bootstrap This starts `git` commands inside `GitInfo`and the submodule updates in parallel. Git should already perform internal locking in cases where it needs to serialize a modification. ``` OLD Benchmark #1: ./x check core Time (mean ± σ): 608.7 ms ± 4.4 ms [User: 368.3 ms, System: 455.1 ms] Range (min … max): 602.3 ms … 618.8 ms 10 runs NEW Benchmark #1: ./x check core Time (mean ± σ): 462.8 ms ± 2.6 ms [User: 350.2 ms, System: 485.1 ms] Range (min … max): 457.5 ms … 465.6 ms 10 runs ``` This should help with the rust-analyzer setup which issues many individual `./x check` calls. There's more that could be done but these were the lowest-hanging fruits that I saw.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Stabilization report
API summary
i*
:n.isqrt()
(panics ifn
is negative) andn.checked_isqrt()
(None
ifn
is negative) (accepted by @dtolnay without ACP)u*
:n.isqrt()
(accepted by @dtolnay without ACP)NonZeroU*
:n.isqrt()
(ACP accepted)const fn
const fn
concernsIf a better implementation comes along that can't be
const fn
,const_eval_select
can be used to combine the better implementation with the current implementation to maintainconst fn
.Experience report
These projects use the
isqrt
feature (note that only five pages of results are allowed, so there may be more uses):#![feature(isqrt)]
.isqrt()
.checked_isqrt()
(right now, there appear to be very few uses that aren'trust-lang/rust
forks)Standard libraries of programming languages
The following languages have an integer square root function in the standard library: Chapel, Common Lisp, Crystal, Julia, Maple, Python 3, Racket, Ruby, SageMath, Scheme, and Tcl.
Rust crates
integer-sqrt
(6.3 million downloads of all versions; GitHub search)num-bigint
(133 million downloads of all versions)rug
(590 thousand downloads of all versions)Other languages
GMP library
Implementation history
isqrt
andchecked_isqrt
methods added to primitive types during 1.74 nightly: Add "integer square root" method to integer primitive types rust-lang/rust#116176isqrt
method on unsignedNonZero
types during 1.80 nightly: ACP:isqrt
forNonZero<uN>
rust-lang/libs-team#391isqrt
method added to unsignedNonZero
types during 1.81 nightly: Addisqrt
toNonZero<uN>
rust-lang/rust#126199checked_isqrt
andisqrt
methods rust-lang/rust#128166Performance
The current implementation is based on a lookup table for 8-bit integers and "unrolled" recursive Karatsuba square root for larger types. The performance is decent, but it can be beaten with
f32::sqrt
andf64::sqrt
for 16-bit through 64-bit integers (64 bit needs a slight adjustment to correct for imprecision). Even 128-bit integers could be sped up by using Karatsuba square root combined with the 64-bit floating point-based algorithm.It isn't done that way because:
core
library code via acfg
setting or similar.Ignoring floating point implementations, there are also faster algorithms that use only integer operations (for example the GMP library has faster implementations). I'm not a lawyer, but I believe that porting those to Rust would be considered a derivative work, and thus require adherence to the original code's license. Since the Rust standard library appears to be dual licensed under Apache 2.0 and MIT and since GMP is licensed under both LGPL 2 and LGPL 3, we'd have to complicate our licensing situation to port the GMP code.
Is there anything else that needs to be taken care of before the
isqrt
feature is stabilized?The text was updated successfully, but these errors were encountered: