Skip to content

/rust

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

Use ScalarPair for tagged enums #49420

Merged
merged 1 commit into from Apr 27, 2018

Conversation

Reviewers

@eddyb eddyb

@frewsxcv frewsxcv

Assignees

@eddyb eddyb

Labels
S-waiting-on-bors
Projects
None yet
Milestone
No milestone
10 participants
@nox @rust-highfive @eddyb @rkruppe @shepmaster @TimNN @bors @kennytm @frewsxcv @petrochenkov
@nox
Copy link
Contributor

nox commented Mar 27, 2018

No description provided.

@rust-highfive rust-highfive assigned petrochenkov Mar 27, 2018

@rust-highfive

This comment has been minimized.

Sign in to view
Copy link
Collaborator

rust-highfive commented Mar 27, 2018

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @petrochenkov (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review label Mar 27, 2018

@frewsxcv

frewsxcv reviewed Mar 27, 2018
View changes

src/librustc/ty/layout.rs Outdated
FieldPlacement::Arbitrary { ref offsets, .. } => offsets,
_ => bug!(),
};
let mut fields = field_layouts.iter().zip(offsets).filter(|p| !p.0.is_zst());

This comment has been minimized.

Sign in to view
Copy link
@frewsxcv

frewsxcv Mar 27, 2018

Member

hi nox ❤️

tidy error: /checkout/src/librustc/ty/layout.rs:1682: line longer than 100 chars

This comment has been minimized.

Sign in to view
Copy link
@nox

nox Mar 27, 2018

Author Contributor

Fixed! Thank you.

@nox nox force-pushed the nox:enum-scalarpair branch from 7c50101 to 97543cd Mar 27, 2018

@petrochenkov petrochenkov assigned eddyb and unassigned petrochenkov Mar 27, 2018

@nox nox force-pushed the nox:enum-scalarpair branch from 97543cd to b168f84 Mar 27, 2018

@eddyb

eddyb reviewed Mar 28, 2018
View changes

src/test/codegen/function-arguments.rs Outdated
@@ -139,6 +139,12 @@ pub fn return_slice(x: &[u16]) -> &[u16] {
x
}

// CHECK: i64 @enum_id(i64)

This comment has been minimized.

Sign in to view
Copy link
@eddyb

eddyb Mar 28, 2018

Member

Isn't this pre-existing? You'd have to take in two arguments and return { i32, i32 } for the ScalarPair to be there.

This comment has been minimized.

Sign in to view
Copy link
@nox

nox Mar 28, 2018 •
edited

Author Contributor

I see. I changed it to Result<i64, i64>.

@nox nox force-pushed the nox:enum-scalarpair branch from b168f84 to f4d784c Mar 28, 2018

@eddyb

eddyb reviewed Mar 28, 2018
View changes

src/test/codegen/function-arguments.rs Outdated
@@ -139,6 +139,12 @@ pub fn return_slice(x: &[u16]) -> &[u16] {
x
}

// CHECK: { i64, i64 } @enum_id(i64 %x.0, i64 %x.1)
#[no_mangle]
pub fn enum_id(x: Result<i64, i64>) -> Result<i64, i64> {

This comment has been minimized.

Sign in to view
Copy link
@eddyb

eddyb Mar 28, 2018

Member

Why does Option around this create problems? Does it require the combination of your 2 PRs? Can we wait to land one and actually have an Option<Result<i64, i64>> test too?

This comment has been minimized.

Sign in to view
Copy link
@nox

nox Mar 28, 2018

Author Contributor

My other PR landed, let me rebase this one.

This comment has been minimized.

Sign in to view
Copy link
@nox

nox Mar 28, 2018

Author Contributor

Indeed, with the combination of the two optimisations, Option<Result<i64, i64>> also ends up returned as { i64, i64 }.
@eddyb

This comment has been minimized.

Sign in to view
Copy link
Member

eddyb commented Mar 28, 2018

r? @eddyb
@nox

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

nox commented Mar 28, 2018

Cc @rkruppe.

For #49437, this produces:

; bar::foo
; Function Attrs: norecurse nounwind readnone uwtable
define { i32, i32 } @_ZN3bar3foo17h361da50dc86c3b65E(i32, i32) unnamed_addr #0 {
start:
  %switch = icmp eq i32 %0, 0
  %. = zext i1 %switch to i32
  %2 = insertvalue { i32, i32 } undef, i32 %., 0
  %3 = insertvalue { i32, i32 } %2, i32 %1, 1
  ret { i32, i32 } %3
}

; bar::bar
; Function Attrs: norecurse nounwind readnone uwtable
define { i32, i32 } @_ZN3bar3bar17h01d34a35402b7c22E(i32 %x.0, i32 %x.1) unnamed_addr #0 {
start:
  %switch.i = icmp ne i32 %x.0, 0
  %. = zext i1 %switch.i to i32
  %0 = insertvalue { i32, i32 } undef, i32 %., 0
  %1 = insertvalue { i32, i32 } %0, i32 %x.1, 1
  ret { i32, i32 } %1
}

For #38349, this produces:

; Function Attrs: norecurse nounwind readnone uwtable
define { i32, i32 } @id_result(i32, i32) unnamed_addr #0 {
start:
  %switch = icmp ne i32 %0, 0
  %. = zext i1 %switch to i32
  %2 = insertvalue { i32, i32 } undef, i32 %., 0
  %3 = insertvalue { i32, i32 } %2, i32 %1, 1
  ret { i32, i32 } %3
}

@eddyb says it's an LLVM limitation: it can't put range metadata on arguments.

@nox nox force-pushed the nox:enum-scalarpair branch from f4d784c to 54c29dc Mar 28, 2018

@rkruppe

This comment has been minimized.

Sign in to view
Copy link
Member

rkruppe commented Mar 28, 2018

@eddyb says it's an LLVM limitation: it can't put range metadata on arguments.

That matches my knowledge. We could add an assume but these tend to make compile time worse. Otherwise, the only option I see right now is extending LLVM to have range metadata on arguments — a quick search in the list archives tells me people have wanted that for a while and there are no conceptual challenges, someone just needs to implement it.

@nox nox referenced this pull request Mar 30, 2018

Closed

Swap the 2 variants of Option<T> #49499

@shepmaster

This comment has been minimized.

Sign in to view
Copy link
Member

shepmaster commented Apr 7, 2018

Ping from triage, @eddyb — it looks like there are new commits for you.
@eddyb

eddyb reviewed Apr 10, 2018
View changes

src/librustc/ty/layout.rs Outdated
}
}
if let Some((prim, _)) = common_prim {
Abi::ScalarPair(discr.clone(), scalar_unit(prim))

This comment has been minimized.

Sign in to view
Copy link
@eddyb

eddyb Apr 10, 2018

Member

You have to do the dance where you use scalar_pair and check that the offset inside common_prim matches that computed by scalar_pair itself.

This comment has been minimized.

Sign in to view
Copy link
@nox

nox Apr 10, 2018

Author Contributor

Isn't that what I do line 1717?

This comment has been minimized.

Sign in to view
Copy link
@eddyb

eddyb Apr 10, 2018

Member

You check that they all have the same offset, but that might still not be the same as the offset that scalar_pair would compute for the second scalar (the one from common_prim).
@TimNN

This comment has been minimized.

Sign in to view
Copy link
Contributor

TimNN commented Apr 11, 2018

Your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log. 
Resolving deltas: 100% (610777/610777), completed with 4838 local objects.
---
[00:00:48] configure: rust.quiet-tests     := True
---
[00:45:11] ..............................................................................i.....................
[00:45:17] .....................i..............................................................................
---
[00:45:58] i..........................................................................i........................
---
[00:46:53] .............................................i......................................................
---
[00:50:45] .............................i......................................................................
[00:50:58] ..............................................................i.....................................
[00:51:13] ...............................................i....................................................
[00:51:33] ....................................................................................................
[00:51:54] ....................................................................................................
[00:52:15] ....................................................................................................
[00:52:39] ...i...............................................................................................i
[00:53:07] ......................................................................................test [run-pass] run-pass/mir_heavy_promoted.rs has been running for over 60 seconds
[00:53:15] ..............
[00:53:44] ....................................................................................................
[00:54:18] .................................................................ii.................................
[00:55:05] ............................i....................................................i.ii......test [run-pass] run-pass/saturating-float-casts.rs has been running for over 60 seconds
[00:55:08] .........
[00:55:50] .........................................................................................iiiiiii....
---
[00:58:00] ...................i............................................................ii.iii..............
[00:58:07] ....................................................................................................
[00:58:15] .........i..............................i...........................................................
[00:58:22] ....................................................................................................
[00:58:28] .....................i..............................................................................
[00:58:36] ....................................................................................................
[00:58:45] ....................................................................................................
[00:58:55] ....................................................................................................
[00:59:05] ....................................................................................................
[00:59:17] ....................................................................................................
[00:59:27] ..............i.....................................................................................
[00:59:36] ..................i..ii.............................................................................
[00:59:45] ....................................................................................................
[00:59:54] ....................................................................................................
[01:00:03] .....................................................................................i..............
[01:00:12] ...............................i....................................................................
---
[01:00:46] ...........................i........................................................................
[01:00:47] ....................................................................i...............................
[01:00:48] ................i.......................................................
---
[01:01:03] .............i........................
---
[01:01:33] thread 'main' panicked at 'Some tests failed', tools/compiletest/src/main.rs:486:22
[01:01:33] i...i..ii....i.............ii.........iii......i..i...i...ii..i.Fi..ii.....
[01:01:33] failures:
[01:01:33]
[01:01:33] ---- [codegen] codegen/repeat-trusted-len.rs stdout ----
[01:01:33]
[01:01:33] error: verification with 'FileCheck' failed
[01:01:33] status: exit code: 1
[01:01:33] command: "/usr/lib/llvm-3.9/bin/FileCheck" "--input-file" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/codegen/repeat-trusted-len.ll" "/checkout/src/test/codegen/repeat-trusted-len.rs"
---
[01:01:33] /checkout/src/test/codegen/repeat-trusted-len.rs:26:11: error: expected string not found in input
[01:01:33] // CHECK: call void @llvm.memset.p0i8.[[USIZE]](i8* {{(nonnull )?}}%{{[0-9]+}}, i8 42, [[USIZE]] 100000, i32 1, i1 false)
[01:01:33]           ^
[01:01:33] /checkout/obj/build/x86_64-unknown-linux-gnu/test/codegen/repeat-trusted-len.ll:43:33: note: scanning from here
[01:01:33] define void @repeat_take_collect(%"alloc::vec::Vec<u8>"* noalias nocapture sret dereferenceable(24)) unnamed_addr #2 personality i32 (i32, i32, i64, %"unwind::libunwind::_Unwind_Exception"*, %"unwind::libunwind::_Unwind_Context"*)* @rust_eh_personality {
[01:01:33]                                 ^
[01:01:33] /checkout/obj/build/x86_64-unknown-linux-gnu/test/codegen/repeat-trusted-len.ll:43:33: note: with variable "USIZE" equal to "i64"
[01:01:33] define void @repeat_take_collect(%"alloc::vec::Vec<u8>"* noalias nocapture sret dereferenceable(24)) unnamed_addr #2 personality i32 (i32, i32, i64, %"unwind::libunwind::_Unwind_Exception"*, %"unwind::libunwind::_Unwind_Context"*)* @rust_eh_personality {
[01:01:33]                                 ^
[01:01:33] /checkout/obj/build/x86_64-unknown-linux-gnu/test/codegen/repeat-trusted-len.ll:43:33: note: with variable "USIZE" equal to "i64"
[01:01:33] define void @repeat_take_collect(%"alloc::vec::Vec<u8>"* noalias nocapture sret dereferenceable(24)) unnamed_addr #2 personality i32 (i32, i32, i64, %"unwind::libunwind::_Unwind_Exception"*, %"unwind::libunwind::_Unwind_Context"*)* @rust_eh_personality {
[01:01:33]                                 ^
[01:01:33]
[01:01:33] ------------------------------------------
[01:01:33]
[01:01:33] thread '[codegen] codegen/repeat-trusted-len.rs' panicked at 'explicit panic', tools/compiletest/src/runtest.rs:2919:9
---
[01:01:33] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/compiletest" "--compile-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib" "--run-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/x86_64-unknown-linux-gnu/lib" "--rustc-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "--src-base" "/checkout/src/test/codegen" "--build-base" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/codegen" "--stage-id" "stage2-x86_64-unknown-linux-gnu" "--mode" "codegen" "--target" "x86_64-unknown-linux-gnu" "--host" "x86_64-unknown-linux-gnu" "--llvm-filecheck" "/usr/lib/llvm-3.9/bin/FileCheck" "--host-rustcflags" "-Crpath -O -Zmiri -Zunstable-options " "--target-rustcflags" "-Crpath -O -Zmiri -Zunstable-options  -Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--docck-python" "/usr/bin/python2.7" "--lldb-python" "/usr/bin/python2.7" "--gdb" "/usr/bin/gdb" "--quiet" "--llvm-version" "3.9.1\n" "--system-llvm" "--cc" "" "--cxx" "" "--cflags" "" "--llvm-components" "" "--llvm-cxxflags" "" "--adb-path" "adb" "--adb-test-dir" /incremental/bootstrap-351vorei3hhuv
121664 ./obj/build/bootstrap/debug/incremental/bootstrap-351vorei3hhuv/s-f003e6vq61-1ve3dbp-179sb4yv5t1yk
---
102728 ./obj/build/x86_64-unknown-linux-gnu/stage1-std/x86_64-unknown-linux-gnu/release/incremental/core-31lccp6wy7orz/s-f004h74421-1efcbrl-6xnnylbd6ghy
100676 ./obj/build/bootstrap/debug/incremental/bootstrap-zki88qpb4pjm
100672 ./obj/build/bootstrap/debug/incremental/bootstrap-zki88qpb4pjm/s-f004jp485c-4w7z5g-1raxz7drp4c1e
---
78276 ./obj/build/x86_64-unknownvis_time:start:1323fc30
$ find $HOME/Library/Logs/DiagnosticReports -type f -name '*.crash' -not -name '*.stage2-*.crash' -not -name 'com.apple.CoreSimulator.CoreSimulatorService-*.crash' -exec printf travis_fold":start:crashlog\n\033[31;1m%s\033[0m\n" {} \; -exec head -750 {} \; -exec echo travis_fold":"end:crashlog \; || true
find: `/home/travis/Library/Logs/DiagnosticReports': No such file or directory
travis_time:end:1323fc30:start=1523410292034665325,finish=1523410292041355443,duration=6690118
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:06c862a1
$ dmesg | grep -i kill
[   10.735799] init: failsafe main process (1093) killed by TERM signal

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)
@nox

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

nox commented Apr 11, 2018

Can someone reproduce this on Linux? I can't reproduce the failure, but it seems to me like the generated code with this PR is way worse.
@nox

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

nox commented Apr 11, 2018

Disregard my last comment, it took me an embarrassingly long time to notice the test changed upstream.
@nox

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

nox commented Apr 11, 2018

https://gist.github.com/nox/c665636b476a546299f0ad2c41688685

@nox nox force-pushed the nox:enum-scalarpair branch from f0daf55 to cded855 Apr 12, 2018

@rust-highfive

This comment has been minimized.

Sign in to view
Copy link
Collaborator

rust-highfive commented Apr 12, 2018

Your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log. 
Resolving deltas: 100% (614293/614293), completed with 4902 local objects.
---
[00:00:51] configure: rust.quiet-tests     := True
---
[00:44:32] ...............................................................................i....................
[00:44:38] ......................i.............................................................................
---
[00:45:19] ..i...........................................................................i.....................
---
[00:46:13] .............................................i......................................................
---
[00:49:59] .............................i......................................................................
[00:50:12] ..............................................................i.....................................
[00:50:26] ................................................i...................................................
[00:50:45] ....................................................................................................
[00:51:05] ....................................................................................................
[00:51:25] ....................................................................................................
[00:51:48] ......i.............................................................................................
[00:52:19] ..i...........................................................................................test [run-pass] run-pass/mir_heavy_promoted.rs has been running for over 60 seconds
[00:52:22] ......
[00:52:52] ....................................................................................................
[00:53:24] ....................................................................ii..............................
[00:54:13] ...............................i....................................................i.ii...........test [run-pass] run-pass/saturating-float-casts.rs has been running for over 60 seconds
[00:54:13] .
[00:54:53] ............................................................................................iiiiiii.
---
[00:57:05] ...................i............................................................ii.iii..............
[00:57:12] ....................................................................................................
[00:57:20] .........i..............................i...........................................................
[00:57:27] ....................................................................................................
[00:57:33] .....................i..............................................................................
[00:57:41] ....................................................................................................
[00:57:50] ....................................................................................................
[00:57:59] ....................................................................................................
[00:58:09] ....................................................................................................
[00:58:21] ....................................................................................................
[00:58:30] ..............i.....................................................................................
[00:58:39] ..................i..ii.............................................................................
[00:58:47] ....................................................................................................
[00:58:56] ....................................................................................................
[00:59:04] .....................................................................................i..............
[00:59:14] ...............................i....................................................................
---
[00:59:46] ...........................i........................................................................
[00:59:47] ....................................................................i...............................
[00:59:48] ................i.......................................................
---
[01:00:03] .............i........................
---
[01:00:33] i...i..ii....i.............ii.........iii......i..i...i...ii..i.Fi..ii.....
[01:00:33] failures:
[01:00:33] thread 'main' panicked at 'Some tests failed', tools/compiletest/src/main.rs:486:22
[01:00:33]
[01:00:33] ---- [codegen] codegen/repeat-trusted-len.rs stdout ----
[01:00:33]
[01:00:33] error: verification with 'FileCheck' failed
[01:00:33] status: exit code: 1
[01:00:33] command: "/usr/lib/llvm-3.9/bin/FileCheck" "--input-file" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/codegen/repeat-trusted-len.ll" "/checkout/src/test/codegen/repeat-trusted-len.rs"
---
[01:00:33] /checkout/src/test/codegen/repeat-trusted-len.rs:26:11: error: expected string not found in input
[01:00:33] // CHECK: call void @llvm.memset.p0i8.[[USIZE]](i8* {{(nonnull )?}}%{{[0-9]+}}, i8 42, [[USIZE]] 100000, i32 1, i1 false)
[01:00:33]           ^
[01:00:33] /checkout/obj/build/x86_64-unknown-linux-gnu/test/codegen/repeat-trusted-len.ll:43:33: note: scanning from here
[01:00:33] define void @repeat_take_collect(%"alloc::vec::Vec<u8>"* noalias nocapture sret dereferenceable(24)) unnamed_addr #2 personality i32 (i32, i32, i64, %"unwind::libunwind::_Unwind_Exception"*, %"unwind::libunwind::_Unwind_Context"*)* @rust_eh_personality {
[01:00:33]                                 ^
[01:00:33] /checkout/obj/build/x86_64-unknown-linux-gnu/test/codegen/repeat-trusted-len.ll:43:33: note: with variable "USIZE" equal to "i64"
[01:00:33] define void @repeat_take_collect(%"alloc::vec::Vec<u8>"* noalias nocapture sret dereferenceable(24)) unnamed_addr #2 personality i32 (i32, i32, i64, %"unwind::libunwind::_Unwind_Exception"*, %"unwind::libunwind::_Unwind_Context"*)* @rust_eh_personality {
[01:00:33]                                 ^
[01:00:33] /checkout/obj/build/x86_64-unknown-linux-gnu/test/codegen/repeat-trusted-len.ll:43:33: note: with variable "USIZE" equal to "i64"
[01:00:33] define void @repeat_take_collect(%"alloc::vec::Vec<u8>"* noalias nocapture sret dereferenceable(24)) unnamed_addr #2 personality i32 (i32, i32, i64, %"unwind::libunwind::_Unwind_Exception"*, %"unwind::libunwind::_Unwind_Context"*)* @rust_eh_personality {
[01:00:33]                                 ^
[01:00:33]
[01:00:33] ------------------------------------------
[01:00:33]
[01:00:33] thread '[codegen] codegen/repeat-trusted-len.rs' panicked at 'explicit panic', tools/compiletest/src/runtest.rs:2919:9
---
[01:00:33] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/compiletest" "--compile-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib" "--run-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/x86_64-unknown-linux-gnu/lib" "--rustc-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "--src-base" "/checkout/src/test/codegen" "--build-base" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/codegen" "--stage-id" "stage2-x86_64-unknown-linux-gnu" "--mode" "codegen" "--target" "x86_64-unknown-linux-gnu" "--host" "x86_64-unknown-linux-gnu" "--llvm-filecheck" "/usr/lib/llvm-3.9/bin/FileCheck" "--host-rustcflags" "-Crpath -O -Zmiri -Zunstable-options " "--target-rustcflags" "-Crpath -O -Zmiri -Zunstable-options  -Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--docck-python" "/usr/bin/python2.7" "--lldb-python" "/usr/bin/python2.7" "--gdb" "/usr/bin/gdb" "--quiet" "--llvm-version" "3.9.1\n" "--system-llvm" "--cc" "" "--cxx" "" "--cflags" "" "--llvm-components" "" "--llvm-cxxflags" "" "--adb-path" "adb" "--adb-test-dir" "/data/tmp/work" "--android-cross-path" "" "--color" "always"
[01:00:33] expected success, got: exit code: 101
[01:00:33]
[01:00:33]
[01:00:33] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test
[01:00:33] Build completed unsuccessfully in 0:17:15
[01:00:33] make: *** [check] Error 1
[01:00:33] Makefile:58: recipe for target 'check' failed
---
$ ls -lat $HOME/Library/Logs/DiagnosticReports/
ls: cannot access /home/travis/Library/Logs/DiagnosticReports/: No such file or directory
travis_time:end:1b90a66f:start=1523528238823343994,finish=1523528238833911106,duration=10567112
travis_fold:end:after_failure.2
travis_fold:start:after_failure.3
travis_time:start:207c6f1d
$ find $HOME/Library/Logs/DiagnosticReports -type f -name '*.crash' -not -name '*.stage2-*.crash' -not -name 'com.apple.CoreSimulator.CoreSimulatorService-*.crash' -exec printf travis_fold":start:crashlog\n\033[31;1m%s\033[0m\n" {} \; -exec head -750 {} \; -exec echo travis_fold":"end:crashlog \; || true
find: `/home/travis/Library/Logs/DiagnosticReports': No such file or directory
travis_time:end:207c6f1d:start=1523528238838972343,finish=1523528238844743600,duration=5771257
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:0b05e372
$ dmesg | grep -i kill
[   10.474070] init: failsafe main process (1092) killed by TERM signal

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@nox nox referenced this pull request Apr 14, 2018

Closed

Surprising optimization differences between variants of the same code #48200

@nox

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

nox commented Apr 16, 2018

I'll try to reduce the failing test that causes some memset optimisation to not trigger in LLVM.
@bors

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors commented Apr 18, 2018

☔️ The latest upstream changes (presumably #49981) made this pull request unmergeable. Please resolve the merge conflicts.

@nox nox force-pushed the nox:enum-scalarpair branch from cded855 to 3755e0d Apr 18, 2018

@rust-highfive

This comment has been minimized.

Sign in to view
Copy link
Collaborator

rust-highfive commented Apr 18, 2018

Your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log. 
[00:00:47] configure: rust.quiet-tests     := True
---
[00:40:31] .................................................................................i..................
[00:40:36] ........................i...........................................................................
---
[00:41:14] .....................i...........................................................................i..
[00:41:20] ....................................................................................................
[00:41:26] ...........ii.......................................................................................
[00:41:33] ............................................................................................i.......
---
[00:42:05] .............................................i......................................................
---
[00:45:28] ..............................i.....................................................................
[00:45:40] ...............................................................i....................................
[00:45:53] .................................................i..................................................
[00:46:10] ....................................................................................................
[00:46:28] ....................................................................................................
[00:46:46] ....................................................................................................
[00:47:07] ........i...........................................................................................
[00:47:37] ....i...............................................................................................
[00:47:41] ......test [run-pass] run-pass/mir_heavy_promoted.rs has been running for over 60 seconds
[00:48:05] ..............................................................................................
[00:48:30] ......................................................................ii............................
[00:49:17] .................................i....................................................i.ii..........
[00:49:24] .................test [run-pass] run-pass/saturating-float-casts.rs has been running for over 60 seconds
[00:49:54] .............................................................................iiiiii
[00:50:17] i...................................................................................................
---
[00:52:01] ....................i............................................................ii.iii.............
[00:52:08] ....................................................................................................
[00:52:15] .........i..............................i...........................................................
[00:52:21] ....................................................................................................
[00:52:27] ...........i........................................................................................
[00:52:34] ....................................................................................................
[00:52:42] ....................................................................................................
[00:52:51] ....................................................................................................
[00:53:00] ....................................................................................................
[00:53:12] ....................................................................................................
[00:53:19] ....i...............................................................................................
[00:53:27] ........i..ii.......................................................................................
[00:53:36] ....................................................................................................
[00:53:43] ....................................................................................................
[00:53:51] ..........................................................................i.........................
[00:54:00] ....................i...............................................................................
---
[00:54:29] ............................i.......................................................................
[00:54:30] ....................................................................i...............................
[00:54:31] ................i......................................................
---
[00:54:45] .............i........................
---
[00:55:12] i...i..ii....i.............ii.........iii......i..i...i...ii..i.Fi..ii.....
[00:55:12] failures:
[00:55:12]
[00:55:12] ---- [codegen] codegen/repeat-trusted-len.rs stdout ----
[00:55:12]
[00:55:12] error: verification with 'FileCheck' failed
[00:55:12] status: exit code: 1
[00:55:12] command: "/usr/lib/llvm-3.9/bin/FileCheck" "--input-file" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/codegen/repeat-trusted-len.ll" "/checkout/src/test/codegen/repeat-trusted-len.rs"
---
[00:55:12] /checkout/src/test/codegen/repeat-trusted-len.rs:26:11: error: expected string not found in input
[00:55:12] // CHECK: call void @llvm.memset.p0i8.[[USIZE]](i8* {{(nonnull )?}}%{{[0-9]+}}, i8 42, [[USIZE]] 100000, i32 1, i1 false)
[00:55:12]           ^
[00:55:12] /checkout/obj/build/x86_64-unknown-linux-gnu/test/codegen/repeat-trusted-len.ll:17:33: note: scanning from here
[00:55:12] define void @repeat_take_collect(%"alloc::vec::Vec<u8>"* noalias nocapture sret dereferenceable(24)) unnamed_addr #1 personality i32 (i32, i32, i64, %"unwind::libunwind::_Unwind_Exception"*, %"unwind::libunwind::_Unwind_Context"*)* @rust_eh_personality {
[00:55:12]                                 ^
[00:55:12] /checkout/obj/build/x86_64-unknown-linux-gnu/test/codegen/repeat-trusted-len.ll:17:33: note: with variable "USIZE" equal to "i64"
[00:55:12] define void @repeat_take_collect(%"alloc::vec::Vec<u8>"* noalias nocapture sret dereferenceable(24)) unnamed_addr #1 personality i32 (i32, i32, i64, %"unwind::libunwind::_Unwind_Exception"*, %"unwind::libunwind::_Unwind_Context"*)* @rust_eh_personality {
[00:55:12]                                 ^
[00:55:12] /checkout/obj/build/x86_64-unknown-linux-gnu/test/codegen/repeat-trusted-len.ll:17:33: note: with variable "USIZE" equal to "i64"
[00:55:12] define void @repeat_take_collect(%"alloc::vec::Vec<u8>"* noalias nocapture sret dereferenceable(24)) unnamed_addr #1 personality i32 (i32, i32, i64, %"unwind::libunwind::_Unwind_Exception"*, %"unwind::libunwind::_Unwind_Context"*)* @rust_eh_personality {
[00:55:12]                                 ^
[00:55:12]
[00:55:12] ------------------------------------------
[00:55:12]
[00:55:12] thread '[codegen] codegen/repeat-trusted-len.rs' panicked at 'explicit panic', tools/compiletest/src/runtest.rs:2930:9
---
[00:55:12] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/compiletest" "--compile-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib" "--run-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/x86_64-unknown-linux-gnu/lib" "--rustc-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "--src-base" "/checkout/src/test/codegen" "--build-base" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/codegen" "--stage-id" "stage2-x86_64-unknown-linux-gnu" "--mode" "codegen" "--target" "x86_64-unknown-linux-gnu" "--host" "x86_64-unknown-linux-gnu" "--llvm-filecheck" "/usr/lib/llvm-3.9/bin/FileCheck" "--host-rustcflags" "-Crpath -O -Zunstable-options " "--target-rustcflags" "-Crpath -O -Zunstable-options  -Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--docck-python" "/usr/bin/python2.7" "--lldb-python" "/usr/bin/python2.7" "--gdb" "/usr/bin/gdb" "--quiet" "--llvm-version" "3.9.1\n" "--system-llvm" "--cc" "" "--cxx" "" "--cflags" "" "--llvm-components" "" "--llvm-cxxflags" "" "--adb-path" "adb" "--adb-test-dir" "/data/tmp/work" "--android-cross-path" "" "--color" "always"
[00:55:12] expected success, got: exit code: 101
[00:55:12]
[00:55:12]
[00:55:12] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test
[00:55:12] Build completed unsuccessfully in 0:15:48
[00:55:12] make: *** [check] Error 1
[00:55:12] Makefile:58: recipe for target 'check' failed
---
$ ls -lat $HOME/Library/Logs/DiagnosticReports/
ls: cannot access /home/travis/Library/Logs/DiagnosticReports/: No such file or directory

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)
@nox

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

nox commented Apr 20, 2018

https://botbot.me/mozilla/rustc/2018-04-20/?msg=99190765&page=2
@eddyb

This comment has been minimized.

Sign in to view
Copy link
Member

eddyb commented Apr 26, 2018

Oh, you should use u16 instead of u64, because its alignment (and thus the size of the tag) varies.
@nox
Use ScalarPair for tagged enums
Loading status checks…
3ca6ad9

@nox nox force-pushed the nox:enum-scalarpair branch from 0af1297 to 3ca6ad9 Apr 26, 2018

@nox

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

nox commented Apr 26, 2018

@bors-servo r=eddyb

pub fn enum_id_1(x: Option<Result<u16, u16>>) -> Option<Result<u16, u16>> {
@bors

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors commented Apr 26, 2018

@nox: 🔑 Insufficient privileges: Not in reviewers
@eddyb

This comment has been minimized.

Sign in to view
Copy link
Member

eddyb commented Apr 26, 2018

@bors r+
@bors

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors commented Apr 26, 2018

📌 Commit 3ca6ad9 has been approved by eddyb

@bors bors added S-waiting-on-bors and removed S-waiting-on-review labels Apr 26, 2018

bors added a commit that referenced this pull request Apr 26, 2018

@bors
Auto merge of #49420 - nox:enum-scalarpair, r=eddyb 
Use ScalarPair for tagged enums
Loading status checks…
d1f3dcf
@bors

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors commented Apr 26, 2018

⌛️ Testing commit 3ca6ad9 with merge d1f3dcf...
@bors

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors commented Apr 26, 2018

💔 Test failed - status-appveyor

@bors bors added S-waiting-on-review and removed S-waiting-on-bors labels Apr 26, 2018

@rust-highfive

This comment was marked as resolved.

Sign in to view
Copy link
Collaborator

rust-highfive commented Apr 26, 2018

Your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log. 
Cloning into 'rust-lang/rust'...
travis_time:end:10c408cc:start=1524767179490653927,finish=1524767186190575652,duration=6699921725
$ cd rust-lang/rust
$ git checkout -qf d1f3dcff5e7374bcc97d02309d41aef96a36c02f
fatal: reference is not a tree: d1f3dcff5e7374bcc97d02309d41aef96a36c02f
The command "git checkout -qf d1f3dcff5e7374bcc97d02309d41aef96a36c02f" failed and exited with 128 during .
Your build has been stopped.

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)
@kennytm

This comment has been minimized.

Sign in to view
Copy link
Member

kennytm commented Apr 26, 2018

@bors retry

AppVeyor problem:

Timeout performing HSET cloud_jobs_79, inst: 1, mgr: ProcessReadQueue, err: never, queue: 12, qu: 0, qs: 12, qc: 0, wr: 0, wq: 0, in: 64, ar: 1, clientName: AppVeyor-Appveyor.Worker_IN_2, IOCP: (Busy=2,Free=998,Min=200,Max=1000), WORKER: (Busy=61,Free=32706,Min=200,Max=32767), Local-CPU: 0.25% (Please take a look at this article for some common client-side issues that can cause timeouts: https://github.com/StackExchange/StackExchange.Redis/tree/master/Docs/Timeouts.md)

@bors bors added S-waiting-on-bors and removed S-waiting-on-review labels Apr 26, 2018

@bors

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors commented Apr 27, 2018

⌛️ Testing commit 3ca6ad9 with merge e05b78d...

bors added a commit that referenced this pull request Apr 27, 2018

@bors
Auto merge of #49420 - nox:enum-scalarpair, r=eddyb 
Use ScalarPair for tagged enums
Loading status checks…
e05b78d
@bors

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors commented Apr 27, 2018

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

@bors bors merged commit 3ca6ad9 into rust-lang:master Apr 27, 2018
2 checks passed

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details

@nox nox deleted the nox:enum-scalarpair branch Apr 28, 2018

@pietroalbini pietroalbini referenced this pull request May 1, 2018

Closed

Internal compiler error librustc/ty/layout.rs:1627:25 #50371

This was referenced Jul 3, 2018

Closed
Merged

kennytm added a commit to kennytm/rust that referenced this pull request Jul 21, 2018

@kennytm
Rollup merge of rust-lang#52051 - scottmcm:swap-directly, r=alexcrichton 
mem::swap the obvious way for types smaller than the SIMD optimization's block size

LLVM isn't able to remove the alloca for the unaligned block in the post-SIMD tail in some cases, so doing this helps SRoA work in cases where it currently doesn't.  Found in the `replace_with` RFC discussion.

Examples of the improvements:
<details>
 <summary>swapping `[u16; 3]` takes 1/3 fewer instructions and no stackalloc</summary>

```rust
type Demo = [u16; 3];
pub fn swap_demo(x: &mut Demo, y: &mut Demo) {
    std::mem::swap(x, y);
}
```

nightly:
```asm
_ZN4blah9swap_demo17ha1732a9b71393a7eE:
.seh_proc _ZN4blah9swap_demo17ha1732a9b71393a7eE
	sub	rsp, 32
	.seh_stackalloc 32
	.seh_endprologue
	movzx	eax, word ptr [rcx + 4]
	mov	word ptr [rsp + 4], ax
	mov	eax, dword ptr [rcx]
	mov	dword ptr [rsp], eax
	movzx	eax, word ptr [rdx + 4]
	mov	word ptr [rcx + 4], ax
	mov	eax, dword ptr [rdx]
	mov	dword ptr [rcx], eax
	movzx	eax, word ptr [rsp + 4]
	mov	word ptr [rdx + 4], ax
	mov	eax, dword ptr [rsp]
	mov	dword ptr [rdx], eax
	add	rsp, 32
	ret
	.seh_handlerdata
	.section	.text,"xr",one_only,_ZN4blah9swap_demo17ha1732a9b71393a7eE
	.seh_endproc
```

this PR:
```asm
_ZN4blah9swap_demo17ha1732a9b71393a7eE:
	mov	r8d, dword ptr [rcx]
	movzx	r9d, word ptr [rcx + 4]
	movzx	eax, word ptr [rdx + 4]
	mov	word ptr [rcx + 4], ax
	mov	eax, dword ptr [rdx]
	mov	dword ptr [rcx], eax
	mov	word ptr [rdx + 4], r9w
	mov	dword ptr [rdx], r8d
	ret
```
</details>

<details>
 <summary>`replace_with` optimizes down much better</summary>

Inspired by rust-lang/rfcs#2490,

```rust
fn replace_with<T, F>(x: &mut Option<T>, f: F)
    where F: FnOnce(Option<T>) -> Option<T>
{
    *x = f(x.take());
}

pub fn inc_opt(mut x: &mut Option<i32>) {
    replace_with(&mut x, |i| i.map(|j| j + 1));
}
```

Rust 1.26.0:
```asm
_ZN4blah7inc_opt17heb0acb64c51777cfE:
	mov	rax, qword ptr [rcx]
	movabs	r8, 4294967296
	add	r8, rax
	shl	rax, 32
	movabs	rdx, -4294967296
	and	rdx, r8
	xor	r8d, r8d
	test	rax, rax
	cmove	rdx, rax
	setne	r8b
	or	rdx, r8
	mov	qword ptr [rcx], rdx
	ret
```

Nightly (better thanks to ScalarPair, maybe?):
```asm
_ZN4blah7inc_opt17h66df690be0b5899dE:
	mov	r8, qword ptr [rcx]
	mov	rdx, r8
	shr	rdx, 32
	xor	eax, eax
	test	r8d, r8d
	setne	al
	add	edx, 1
	mov	dword ptr [rcx], eax
	mov	dword ptr [rcx + 4], edx
	ret
```

This PR:
```asm
_ZN4blah7inc_opt17h1426dc215ecbdb19E:
	xor	eax, eax
	cmp	dword ptr [rcx], 0
	setne	al
	mov	dword ptr [rcx], eax
	add	dword ptr [rcx + 4], 1
	ret
```

Where that add is beautiful -- using an addressing mode to not even need to explicitly go through a register -- and the remaining imperfection is well-known (rust-lang#49420 (comment)).
</details>
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
f779234

kennytm added a commit to kennytm/rust that referenced this pull request Jul 21, 2018

@kennytm
Rollup merge of rust-lang#52051 - scottmcm:swap-directly, r=alexcrichton 
mem::swap the obvious way for types smaller than the SIMD optimization's block size

LLVM isn't able to remove the alloca for the unaligned block in the post-SIMD tail in some cases, so doing this helps SRoA work in cases where it currently doesn't.  Found in the `replace_with` RFC discussion.

Examples of the improvements:
<details>
 <summary>swapping `[u16; 3]` takes 1/3 fewer instructions and no stackalloc</summary>

```rust
type Demo = [u16; 3];
pub fn swap_demo(x: &mut Demo, y: &mut Demo) {
    std::mem::swap(x, y);
}
```

nightly:
```asm
_ZN4blah9swap_demo17ha1732a9b71393a7eE:
.seh_proc _ZN4blah9swap_demo17ha1732a9b71393a7eE
	sub	rsp, 32
	.seh_stackalloc 32
	.seh_endprologue
	movzx	eax, word ptr [rcx + 4]
	mov	word ptr [rsp + 4], ax
	mov	eax, dword ptr [rcx]
	mov	dword ptr [rsp], eax
	movzx	eax, word ptr [rdx + 4]
	mov	word ptr [rcx + 4], ax
	mov	eax, dword ptr [rdx]
	mov	dword ptr [rcx], eax
	movzx	eax, word ptr [rsp + 4]
	mov	word ptr [rdx + 4], ax
	mov	eax, dword ptr [rsp]
	mov	dword ptr [rdx], eax
	add	rsp, 32
	ret
	.seh_handlerdata
	.section	.text,"xr",one_only,_ZN4blah9swap_demo17ha1732a9b71393a7eE
	.seh_endproc
```

this PR:
```asm
_ZN4blah9swap_demo17ha1732a9b71393a7eE:
	mov	r8d, dword ptr [rcx]
	movzx	r9d, word ptr [rcx + 4]
	movzx	eax, word ptr [rdx + 4]
	mov	word ptr [rcx + 4], ax
	mov	eax, dword ptr [rdx]
	mov	dword ptr [rcx], eax
	mov	word ptr [rdx + 4], r9w
	mov	dword ptr [rdx], r8d
	ret
```
</details>

<details>
 <summary>`replace_with` optimizes down much better</summary>

Inspired by rust-lang/rfcs#2490,

```rust
fn replace_with<T, F>(x: &mut Option<T>, f: F)
    where F: FnOnce(Option<T>) -> Option<T>
{
    *x = f(x.take());
}

pub fn inc_opt(mut x: &mut Option<i32>) {
    replace_with(&mut x, |i| i.map(|j| j + 1));
}
```

Rust 1.26.0:
```asm
_ZN4blah7inc_opt17heb0acb64c51777cfE:
	mov	rax, qword ptr [rcx]
	movabs	r8, 4294967296
	add	r8, rax
	shl	rax, 32
	movabs	rdx, -4294967296
	and	rdx, r8
	xor	r8d, r8d
	test	rax, rax
	cmove	rdx, rax
	setne	r8b
	or	rdx, r8
	mov	qword ptr [rcx], rdx
	ret
```

Nightly (better thanks to ScalarPair, maybe?):
```asm
_ZN4blah7inc_opt17h66df690be0b5899dE:
	mov	r8, qword ptr [rcx]
	mov	rdx, r8
	shr	rdx, 32
	xor	eax, eax
	test	r8d, r8d
	setne	al
	add	edx, 1
	mov	dword ptr [rcx], eax
	mov	dword ptr [rcx + 4], edx
	ret
```

This PR:
```asm
_ZN4blah7inc_opt17h1426dc215ecbdb19E:
	xor	eax, eax
	cmp	dword ptr [rcx], 0
	setne	al
	mov	dword ptr [rcx], eax
	add	dword ptr [rcx + 4], 1
	ret
```

Where that add is beautiful -- using an addressing mode to not even need to explicitly go through a register -- and the remaining imperfection is well-known (rust-lang#49420 (comment)).
</details>
Verified
This commit was signed with a verified signature.
@kennytm kennytm
GPG key ID: FEF6C8051D0E013C Learn about signing commits
a7cdf9a

bors added a commit that referenced this pull request Jul 22, 2018

@bors
Auto merge of #52051 - scottmcm:swap-directly, r=alexcrichton 
mem::swap the obvious way for types smaller than the SIMD optimization's block size

LLVM isn't able to remove the alloca for the unaligned block in the post-SIMD tail in some cases, so doing this helps SRoA work in cases where it currently doesn't.  Found in the `replace_with` RFC discussion.

Examples of the improvements:
<details>
 <summary>swapping `[u16; 3]` takes 1/3 fewer instructions and no stackalloc</summary>

```rust
type Demo = [u16; 3];
pub fn swap_demo(x: &mut Demo, y: &mut Demo) {
    std::mem::swap(x, y);
}
```

nightly:
```asm
_ZN4blah9swap_demo17ha1732a9b71393a7eE:
.seh_proc _ZN4blah9swap_demo17ha1732a9b71393a7eE
	sub	rsp, 32
	.seh_stackalloc 32
	.seh_endprologue
	movzx	eax, word ptr [rcx + 4]
	mov	word ptr [rsp + 4], ax
	mov	eax, dword ptr [rcx]
	mov	dword ptr [rsp], eax
	movzx	eax, word ptr [rdx + 4]
	mov	word ptr [rcx + 4], ax
	mov	eax, dword ptr [rdx]
	mov	dword ptr [rcx], eax
	movzx	eax, word ptr [rsp + 4]
	mov	word ptr [rdx + 4], ax
	mov	eax, dword ptr [rsp]
	mov	dword ptr [rdx], eax
	add	rsp, 32
	ret
	.seh_handlerdata
	.section	.text,"xr",one_only,_ZN4blah9swap_demo17ha1732a9b71393a7eE
	.seh_endproc
```

this PR:
```asm
_ZN4blah9swap_demo17ha1732a9b71393a7eE:
	mov	r8d, dword ptr [rcx]
	movzx	r9d, word ptr [rcx + 4]
	movzx	eax, word ptr [rdx + 4]
	mov	word ptr [rcx + 4], ax
	mov	eax, dword ptr [rdx]
	mov	dword ptr [rcx], eax
	mov	word ptr [rdx + 4], r9w
	mov	dword ptr [rdx], r8d
	ret
```
</details>

<details>
 <summary>`replace_with` optimizes down much better</summary>

Inspired by rust-lang/rfcs#2490,

```rust
fn replace_with<T, F>(x: &mut Option<T>, f: F)
    where F: FnOnce(Option<T>) -> Option<T>
{
    *x = f(x.take());
}

pub fn inc_opt(mut x: &mut Option<i32>) {
    replace_with(&mut x, |i| i.map(|j| j + 1));
}
```

Rust 1.26.0:
```asm
_ZN4blah7inc_opt17heb0acb64c51777cfE:
	mov	rax, qword ptr [rcx]
	movabs	r8, 4294967296
	add	r8, rax
	shl	rax, 32
	movabs	rdx, -4294967296
	and	rdx, r8
	xor	r8d, r8d
	test	rax, rax
	cmove	rdx, rax
	setne	r8b
	or	rdx, r8
	mov	qword ptr [rcx], rdx
	ret
```

Nightly (better thanks to ScalarPair, maybe?):
```asm
_ZN4blah7inc_opt17h66df690be0b5899dE:
	mov	r8, qword ptr [rcx]
	mov	rdx, r8
	shr	rdx, 32
	xor	eax, eax
	test	r8d, r8d
	setne	al
	add	edx, 1
	mov	dword ptr [rcx], eax
	mov	dword ptr [rcx + 4], edx
	ret
```

This PR:
```asm
_ZN4blah7inc_opt17h1426dc215ecbdb19E:
	xor	eax, eax
	cmp	dword ptr [rcx], 0
	setne	al
	mov	dword ptr [rcx], eax
	add	dword ptr [rcx + 4], 1
	ret
```

Where that add is beautiful -- using an addressing mode to not even need to explicitly go through a register -- and the remaining imperfection is well-known (#49420 (comment)).
</details>
Loading status checks…
c069aa6

kennytm added a commit to kennytm/rust that referenced this pull request Jul 22, 2018

@kennytm
Rollup merge of rust-lang#52051 - scottmcm:swap-directly, r=alexcrichton 
mem::swap the obvious way for types smaller than the SIMD optimization's block size

LLVM isn't able to remove the alloca for the unaligned block in the post-SIMD tail in some cases, so doing this helps SRoA work in cases where it currently doesn't.  Found in the `replace_with` RFC discussion.

Examples of the improvements:
<details>
 <summary>swapping `[u16; 3]` takes 1/3 fewer instructions and no stackalloc</summary>

```rust
type Demo = [u16; 3];
pub fn swap_demo(x: &mut Demo, y: &mut Demo) {
    std::mem::swap(x, y);
}
```

nightly:
```asm
_ZN4blah9swap_demo17ha1732a9b71393a7eE:
.seh_proc _ZN4blah9swap_demo17ha1732a9b71393a7eE
	sub	rsp, 32
	.seh_stackalloc 32
	.seh_endprologue
	movzx	eax, word ptr [rcx + 4]
	mov	word ptr [rsp + 4], ax
	mov	eax, dword ptr [rcx]
	mov	dword ptr [rsp], eax
	movzx	eax, word ptr [rdx + 4]
	mov	word ptr [rcx + 4], ax
	mov	eax, dword ptr [rdx]
	mov	dword ptr [rcx], eax
	movzx	eax, word ptr [rsp + 4]
	mov	word ptr [rdx + 4], ax
	mov	eax, dword ptr [rsp]
	mov	dword ptr [rdx], eax
	add	rsp, 32
	ret
	.seh_handlerdata
	.section	.text,"xr",one_only,_ZN4blah9swap_demo17ha1732a9b71393a7eE
	.seh_endproc
```

this PR:
```asm
_ZN4blah9swap_demo17ha1732a9b71393a7eE:
	mov	r8d, dword ptr [rcx]
	movzx	r9d, word ptr [rcx + 4]
	movzx	eax, word ptr [rdx + 4]
	mov	word ptr [rcx + 4], ax
	mov	eax, dword ptr [rdx]
	mov	dword ptr [rcx], eax
	mov	word ptr [rdx + 4], r9w
	mov	dword ptr [rdx], r8d
	ret
```
</details>

<details>
 <summary>`replace_with` optimizes down much better</summary>

Inspired by rust-lang/rfcs#2490,

```rust
fn replace_with<T, F>(x: &mut Option<T>, f: F)
    where F: FnOnce(Option<T>) -> Option<T>
{
    *x = f(x.take());
}

pub fn inc_opt(mut x: &mut Option<i32>) {
    replace_with(&mut x, |i| i.map(|j| j + 1));
}
```

Rust 1.26.0:
```asm
_ZN4blah7inc_opt17heb0acb64c51777cfE:
	mov	rax, qword ptr [rcx]
	movabs	r8, 4294967296
	add	r8, rax
	shl	rax, 32
	movabs	rdx, -4294967296
	and	rdx, r8
	xor	r8d, r8d
	test	rax, rax
	cmove	rdx, rax
	setne	r8b
	or	rdx, r8
	mov	qword ptr [rcx], rdx
	ret
```

Nightly (better thanks to ScalarPair, maybe?):
```asm
_ZN4blah7inc_opt17h66df690be0b5899dE:
	mov	r8, qword ptr [rcx]
	mov	rdx, r8
	shr	rdx, 32
	xor	eax, eax
	test	r8d, r8d
	setne	al
	add	edx, 1
	mov	dword ptr [rcx], eax
	mov	dword ptr [rcx + 4], edx
	ret
```

This PR:
```asm
_ZN4blah7inc_opt17h1426dc215ecbdb19E:
	xor	eax, eax
	cmp	dword ptr [rcx], 0
	setne	al
	mov	dword ptr [rcx], eax
	add	dword ptr [rcx + 4], 1
	ret
```

Where that add is beautiful -- using an addressing mode to not even need to explicitly go through a register -- and the remaining imperfection is well-known (rust-lang#49420 (comment)).
</details>
Verified
This commit was signed with a verified signature.
@kennytm kennytm
GPG key ID: FEF6C8051D0E013C Learn about signing commits
b954d4d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.