Skip to content
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

Resolve FIXME in libcore/iter/mod.rs #56630

Merged
merged 3 commits into from
Dec 9, 2018
Merged

Conversation

sinkuu
Copy link
Contributor

@sinkuu sinkuu commented Dec 8, 2018

and makes a few improvements.

@rust-highfive
Copy link
Collaborator

r? @kennytm

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 8, 2018
_ => unreachable!(),
}
let iter = &mut self.iter;
self.peeked.get_or_insert_with(|| iter.next()).as_ref()
Copy link
Contributor Author

@sinkuu sinkuu Dec 8, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seemingly LLVM cannot optimize out the _ => unreachable!, so this improves codegen.

Diff of LLVM IR with opt-level=3
pub fn foo(iter: &mut Peekable<vec::IntoIter<i32>>) -> Option<&i32> {
    iter.peek()
}
--- test.old.ll 2018-12-08 22:14:35.424253110 +0900
+++ test.ll     2018-12-08 22:14:43.778324073 +0900
@@ -9,73 +9,56 @@
 %"unwind::libunwind::_Unwind_Exception" = type { [0 x i64], i64, [0 x i64], void (i32, %"unwind::libunwind::_Unwind_Exception"*)*, [0 x i64], [6 x i64], [0 x i64] }
 %"unwind::libunwind::_Unwind_Context" = type { [0 x i8] }
 
-@0 = private unnamed_addr constant <{ [40 x i8] }> <{ [40 x i8] c"internal error: entered unreachable code" }>, align 1
-@1 = private unnamed_addr constant <{ [23 x i8] }> <{ [23 x i8] c"src/libcore/iter/mod.rs" }>, align 1
-@2 = private unnamed_addr constant <{ i8*, [8 x i8], i8*, [16 x i8] }> <{ i8* getelementptr inbounds (<{ [40 x i8] }>, <{ [40 x i8] }>* @0, i32 0, i32 0, i32 0), [8 x i8] c"(\00\00\00\00\00\00\00", i8* getelementptr inbounds (<{ [23 x i8] }>, <{ [23 x i8] }>* @1, i32 0, i32 0, i32 0), [16 x i8] c"\17\00\00\00\00\00\00\00\B4\07\00\00\12\00\00\00" }>, align 8
-
 ; test::foo
-; Function Attrs: nonlazybind uwtable
-define align 4 dereferenceable_or_null(4) i32* @_ZN4test3foo17h48910d518edefbc0E(%"core::iter::Peekable<alloc::vec::IntoIter<i32>>"* dereferenceable(40) %iter) unnamed_addr #0 personality i32 (i32, i32, i64, %"unwind::libunwind::_Unwind_Exception"*, %"unwind::libunwind::_Unwind_Context"*)* @rust_eh_personality {
+; Function Attrs: norecurse nounwind nonlazybind uwtable
+define align 4 dereferenceable_or_null(4) i32* @_ZN4test3foo17h3aaa37f182f5f74fE(%"core::iter::Peekable<alloc::vec::IntoIter<i32>>"* dereferenceable(40) %iter) unnamed_addr #0 personality i32 (i32, i32, i64, %"unwind::libunwind::_Unwind_Exception"*, %"unwind::libunwind::_Unwind_Context"*)* @rust_eh_personality {
 start:
-  %.idx.i = getelementptr %"core::iter::Peekable<alloc::vec::IntoIter<i32>>", %"core::iter::Peekable<alloc::vec::IntoIter<i32>>"* %iter, i64 0, i32 3, i32 0
-  %.idx.val.i = load i32, i32* %.idx.i, align 4
-  %0 = icmp eq i32 %.idx.val.i, 2
-  br i1 %0, label %bb2.i, label %bb4.i
-
-bb2.i:                                            ; preds = %start
-  %1 = getelementptr inbounds %"core::iter::Peekable<alloc::vec::IntoIter<i32>>", %"core::iter::Peekable<alloc::vec::IntoIter<i32>>"* %iter, i64 0, i32 1, i32 7
-  %2 = load i32*, i32** %1, align 8
-  %3 = getelementptr inbounds %"core::iter::Peekable<alloc::vec::IntoIter<i32>>", %"core::iter::Peekable<alloc::vec::IntoIter<i32>>"* %iter, i64 0, i32 1, i32 9
-  %4 = load i32*, i32** %3, align 8
-  %5 = icmp eq i32* %2, %4
-  br i1 %5, label %"_ZN80_$LT$alloc..vec..IntoIter$LT$T$GT$$u20$as$u20$core..iter..iterator..Iterator$GT$4next17hac55a8161e3fb5c3E.exit.i", label %bb2.i.i
+  %0 = getelementptr inbounds %"core::iter::Peekable<alloc::vec::IntoIter<i32>>", %"core::iter::Peekable<alloc::vec::IntoIter<i32>>"* %iter, i64 0, i32 3, i32 0
+  %1 = load i32, i32* %0, align 4, !range !1
+  %2 = icmp eq i32 %1, 2
+  br i1 %2, label %bb2.i.i, label %"start._ZN38_$LT$core..option..Option$LT$T$GT$$GT$18get_or_insert_with17h1b7503b69cd4dde4E.exit_crit_edge.i"
+
+"start._ZN38_$LT$core..option..Option$LT$T$GT$$GT$18get_or_insert_with17h1b7503b69cd4dde4E.exit_crit_edge.i": ; preds = %start
+  %.pre.i = getelementptr inbounds %"core::iter::Peekable<alloc::vec::IntoIter<i32>>", %"core::iter::Peekable<alloc::vec::IntoIter<i32>>"* %iter, i64 0, i32 3, i32 1
+  br label %"_ZN38_$LT$core..iter..Peekable$LT$I$GT$$GT$4peek17hd154f34ce665a0bcE.exit"
 
-bb2.i.i:                                          ; preds = %bb2.i
-  %6 = getelementptr inbounds i32, i32* %2, i64 1
-  store i32* %6, i32** %1, align 8
-  %.val.i.i = load i32, i32* %2, align 4
-  br label %"_ZN80_$LT$alloc..vec..IntoIter$LT$T$GT$$u20$as$u20$core..iter..iterator..Iterator$GT$4next17hac55a8161e3fb5c3E.exit.i"
-
-"_ZN80_$LT$alloc..vec..IntoIter$LT$T$GT$$u20$as$u20$core..iter..iterator..Iterator$GT$4next17hac55a8161e3fb5c3E.exit.i": ; preds = %bb2.i.i, %bb2.i
-  %_0.sroa.4.0.i.i = phi i32 [ %.val.i.i, %bb2.i.i ], [ undef, %bb2.i ]
-  %_0.sroa.0.0.i.i = phi i32 [ 1, %bb2.i.i ], [ 0, %bb2.i ]
-  store i32 %_0.sroa.0.0.i.i, i32* %.idx.i, align 8
-  %7 = getelementptr inbounds %"core::iter::Peekable<alloc::vec::IntoIter<i32>>", %"core::iter::Peekable<alloc::vec::IntoIter<i32>>"* %iter, i64 0, i32 3, i32 1
-  store i32 %_0.sroa.4.0.i.i, i32* %7, align 4
-  br label %bb4.i
-
-bb4.i:                                            ; preds = %"_ZN80_$LT$alloc..vec..IntoIter$LT$T$GT$$u20$as$u20$core..iter..iterator..Iterator$GT$4next17hac55a8161e3fb5c3E.exit.i", %start
-  %8 = phi i32 [ %_0.sroa.0.0.i.i, %"_ZN80_$LT$alloc..vec..IntoIter$LT$T$GT$$u20$as$u20$core..iter..iterator..Iterator$GT$4next17hac55a8161e3fb5c3E.exit.i" ], [ %.idx.val.i, %start ]
-  switch i32 %8, label %bb6.i [
-    i32 2, label %bb5.i
-    i32 0, label %"_ZN38_$LT$core..iter..Peekable$LT$I$GT$$GT$4peek17h89fcfd64b4417881E.exit"
-  ]
-
-bb5.i:                                            ; preds = %bb4.i
-; call core::panicking::panic
-  tail call void @_ZN4core9panicking5panic17h2cd043862cc6dc4aE({ [0 x i64], { [0 x i8]*, i64 }, [0 x i64], { [0 x i8]*, i64 }, [0 x i32], i32, [0 x i32], i32, [0 x i32] }* noalias readonly dereferenceable(40) bitcast (<{ i8*, [8 x i8], i8*, [16 x i8] }>* @2 to { [0 x i64], { [0 x i8]*, i64 }, [0 x i64], { [0 x i8]*, i64 }, [0 x i32], i32, [0 x i32], i32, [0 x i32] }*))
-  unreachable
-
-bb6.i:                                            ; preds = %bb4.i
+bb2.i.i:                                          ; preds = %start
+  %3 = getelementptr inbounds %"core::iter::Peekable<alloc::vec::IntoIter<i32>>", %"core::iter::Peekable<alloc::vec::IntoIter<i32>>"* %iter, i64 0, i32 1, i32 7
+  %4 = load i32*, i32** %3, align 8
+  %5 = getelementptr inbounds %"core::iter::Peekable<alloc::vec::IntoIter<i32>>", %"core::iter::Peekable<alloc::vec::IntoIter<i32>>"* %iter, i64 0, i32 1, i32 9
+  %6 = load i32*, i32** %5, align 8
+  %7 = icmp eq i32* %4, %6
+  br i1 %7, label %"_ZN38_$LT$core..iter..Peekable$LT$I$GT$$GT$4peek28_$u7b$$u7b$closure$u7d$$u7d$17h5d8c39d83231aa35E.exit.i.i", label %bb2.i.i.i.i
+
+bb2.i.i.i.i:                                      ; preds = %bb2.i.i
+  %8 = getelementptr inbounds i32, i32* %4, i64 1
+  store i32* %8, i32** %3, align 8
+  %.val.i.i.i.i = load i32, i32* %4, align 4
+  br label %"_ZN38_$LT$core..iter..Peekable$LT$I$GT$$GT$4peek28_$u7b$$u7b$closure$u7d$$u7d$17h5d8c39d83231aa35E.exit.i.i"
+
+"_ZN38_$LT$core..iter..Peekable$LT$I$GT$$GT$4peek28_$u7b$$u7b$closure$u7d$$u7d$17h5d8c39d83231aa35E.exit.i.i": ; preds = %bb2.i.i.i.i, %bb2.i.i
+  %_0.sroa.4.0.i.i.i.i = phi i32 [ %.val.i.i.i.i, %bb2.i.i.i.i ], [ undef, %bb2.i.i ]
+  %_0.sroa.0.0.i.i.i.i = phi i32 [ 1, %bb2.i.i.i.i ], [ 0, %bb2.i.i ]
+  store i32 %_0.sroa.0.0.i.i.i.i, i32* %0, align 4
   %9 = getelementptr inbounds %"core::iter::Peekable<alloc::vec::IntoIter<i32>>", %"core::iter::Peekable<alloc::vec::IntoIter<i32>>"* %iter, i64 0, i32 3, i32 1
-  br label %"_ZN38_$LT$core..iter..Peekable$LT$I$GT$$GT$4peek17h89fcfd64b4417881E.exit"
+  store i32 %_0.sroa.4.0.i.i.i.i, i32* %9, align 4
+  br label %"_ZN38_$LT$core..iter..Peekable$LT$I$GT$$GT$4peek17hd154f34ce665a0bcE.exit"
 
-"_ZN38_$LT$core..iter..Peekable$LT$I$GT$$GT$4peek17h89fcfd64b4417881E.exit": ; preds = %bb4.i, %bb6.i
-  %_0.0.i = phi i32* [ %9, %bb6.i ], [ null, %bb4.i ]
-  ret i32* %_0.0.i
+"_ZN38_$LT$core..iter..Peekable$LT$I$GT$$GT$4peek17hd154f34ce665a0bcE.exit": ; preds = %"start._ZN38_$LT$core..option..Option$LT$T$GT$$GT$18get_or_insert_with17h1b7503b69cd4dde4E.exit_crit_edge.i", %"_ZN38_$LT$core..iter..Peekable$LT$I$GT$$GT$4peek28_$u7b$$u7b$closure$u7d$$u7d$17h5d8c39d83231aa35E.exit.i.i"
+  %.pre-phi.i = phi i32* [ %.pre.i, %"start._ZN38_$LT$core..option..Option$LT$T$GT$$GT$18get_or_insert_with17h1b7503b69cd4dde4E.exit_crit_edge.i" ], [ %9, %"_ZN38_$LT$core..iter..Peekable$LT$I$GT$$GT$4peek28_$u7b$$u7b$closure$u7d$$u7d$17h5d8c39d83231aa35E.exit.i.i" ]
+  %10 = phi i32 [ %1, %"start._ZN38_$LT$core..option..Option$LT$T$GT$$GT$18get_or_insert_with17h1b7503b69cd4dde4E.exit_crit_edge.i" ], [ %_0.sroa.0.0.i.i.i.i, %"_ZN38_$LT$core..iter..Peekable$LT$I$GT$$GT$4peek28_$u7b$$u7b$closure$u7d$$u7d$17h5d8c39d83231aa35E.exit.i.i" ]
+  %switch.i.i = icmp eq i32 %10, 1
+  %_0.0.i.i = select i1 %switch.i.i, i32* %.pre-phi.i, i32* null
+  ret i32* %_0.0.i.i
 }
 
 ; Function Attrs: nonlazybind uwtable
 declare i32 @rust_eh_personality(i32, i32, i64, %"unwind::libunwind::_Unwind_Exception"*, %"unwind::libunwind::_Unwind_Context"*) unnamed_addr #1
 
-; core::panicking::panic
-; Function Attrs: cold noinline noreturn nonlazybind uwtable
-declare void @_ZN4core9panicking5panic17h2cd043862cc6dc4aE({ [0 x i64], { [0 x i8]*, i64 }, [0 x i64], { [0 x i8]*, i64 }, [0 x i32], i32, [0 x i32], i32, [0 x i32] }* noalias readonly dereferenceable(40)) unnamed_addr #2
-
-attributes #0 = { nonlazybind uwtable "probe-stack"="__rust_probestack" }
+attributes #0 = { norecurse nounwind nonlazybind uwtable "probe-stack"="__rust_probestack" }
 attributes #1 = { nonlazybind uwtable "probe-stack"="__rust_probestack" "target-cpu"="x86-64" }
-attributes #2 = { cold noinline noreturn nonlazybind uwtable "probe-stack"="__rust_probestack" }
 
 !llvm.module.flags = !{!0}
 
 !0 = !{i32 2, !"RtLibUseGOT", i32 1}
+!1 = !{i32 0, i32 3}

 name                            old ns/iter  new ns/iter  diff ns/iter   diff %  speedup
 iter::bench_cycle_take_ref_sum  927,152      927,194                42    0.00%   x 1.00
 iter::bench_cycle_take_sum      938,129      603,492          -334,637  -35.67%   x 1.55
@kennytm
Copy link
Member

kennytm commented Dec 8, 2018

Thanks!

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Dec 8, 2018

📌 Commit 5728a04 has been approved by kennytm

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 8, 2018
@bors
Copy link
Contributor

bors commented Dec 9, 2018

⌛ Testing commit 5728a04 with merge b7da2c6...

bors added a commit that referenced this pull request Dec 9, 2018
Resolve FIXME in libcore/iter/mod.rs

and makes a few improvements.
@bors
Copy link
Contributor

bors commented Dec 9, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: kennytm
Pushing b7da2c6 to master...

@bors bors merged commit 5728a04 into rust-lang:master Dec 9, 2018
@bluss bluss mentioned this pull request Dec 16, 2018
@bluss
Copy link
Member

bluss commented Dec 16, 2018

See #56883 about a bug we found, if you have time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants