Skip to content

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

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

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
Contributor

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
Collaborator

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
Collaborator

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
Collaborator

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.

# for free to join this conversation on GitHub. Already have an account? # 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