rust icon indicating copy to clipboard operation
rust copied to clipboard

Represent `Result<usize, Box<T>>` as ScalarPair(i64, ptr)

Open erikdesjardins opened this issue 11 months ago • 24 comments

This allows types like Result<usize, std::io::Error> (and integers of differing sign, e.g. Result<u64, i64>) to be passed in a pair of registers instead of through memory, like Result<u64, u64> or Result<Box<T>, Box<U>> are today.

Fixes #97540.

r? @ghost

erikdesjardins avatar Feb 27 '24 05:02 erikdesjardins

@bors try @rust-timer queue

Noratrieb avatar Feb 27 '24 05:02 Noratrieb

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

rust-timer avatar Feb 27 '24 05:02 rust-timer

:hourglass: Trying commit 4dabbcb23b0dc605cdb4aa6f3499d0f053f7b600 with merge 620bc57fd23a71e77bfab2c9db4cb12dff02e970...

bors avatar Feb 27 '24 05:02 bors

The job x86_64-gnu-llvm-16 failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
GITHUB_ACTION=__run_7
GITHUB_ACTIONS=true
GITHUB_ACTION_REF=
GITHUB_ACTION_REPOSITORY=
GITHUB_ACTOR=erikdesjardins
GITHUB_API_URL=https://api.github.com
GITHUB_BASE_REF=master
GITHUB_ENV=/home/runner/work/_temp/_runner_file_commands/set_env_f280974d-38a9-4d4d-a47d-1f068feb06e9
GITHUB_EVENT_NAME=pull_request
---
GITHUB_SERVER_URL=https://github.com
GITHUB_SHA=d3bd4570902aadc8a626a43c590dd0366bea819e
GITHUB_STATE=/home/runner/work/_temp/_runner_file_commands/save_state_f280974d-38a9-4d4d-a47d-1f068feb06e9
GITHUB_STEP_SUMMARY=/home/runner/work/_temp/_runner_file_commands/step_summary_f280974d-38a9-4d4d-a47d-1f068feb06e9
GITHUB_TRIGGERING_ACTOR=erikdesjardins
GITHUB_WORKFLOW_REF=rust-lang/rust/.github/workflows/ci.yml@refs/pull/121668/merge
GITHUB_WORKFLOW_SHA=d3bd4570902aadc8a626a43c590dd0366bea819e
GITHUB_WORKSPACE=/home/runner/work/rust/rust
GOROOT_1_19_X64=/opt/hostedtoolcache/go/1.19.13/x64
---
#12 writing image sha256:a55834afc46e2738063a4a4ea9cba92d568114cc2f665ed5dc94da1d5c167128 done
#12 naming to docker.io/library/rust-ci done
#12 DONE 10.1s
##[endgroup]
Setting extra environment values for docker:  --env ENABLE_GCC_CODEGEN=1 --env GCC_EXEC_PREFIX=/usr/lib/gcc/
[CI_JOB_NAME=x86_64-gnu-llvm-16]
##[group]Clock drift check
  local time: Tue Feb 27 05:21:12 UTC 2024
  network time: Tue, 27 Feb 2024 05:21:12 GMT
  network time: Tue, 27 Feb 2024 05:21:12 GMT
##[endgroup]
sccache: Starting the server...
##[group]Configure the build
configure: processing command line
configure: 
configure: build.configure-args := ['--build=x86_64-unknown-linux-gnu', '--llvm-root=/usr/lib/llvm-16', '--enable-llvm-link-shared', '--set', 'rust.thin-lto-import-instr-limit=10', '--set', 'change-id=99999999', '--enable-verbose-configure', '--enable-sccache', '--disable-manage-submodules', '--enable-locked-deps', '--enable-cargo-native-static', '--set', 'rust.codegen-units-std=1', '--set', 'dist.compression-profile=balanced', '--dist-compression-formats=xz', '--set', 'build.optimized-compiler-builtins', '--disable-dist-src', '--release-channel=nightly', '--enable-debug-assertions', '--enable-overflow-checks', '--enable-llvm-assertions', '--set', 'rust.verify-llvm-ir', '--set', 'rust.codegen-backends=llvm,cranelift,gcc', '--set', 'llvm.static-libstdcpp', '--enable-new-symbol-mangling']
configure: target.x86_64-unknown-linux-gnu.llvm-config := /usr/lib/llvm-16/bin/llvm-config
configure: llvm.link-shared     := True
configure: rust.thin-lto-import-instr-limit := 10
configure: change-id            := 99999999
---
##[endgroup]
Testing GCC stage1 (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
   Compiling y v0.1.0 (/checkout/compiler/rustc_codegen_gcc/build_system)
    Finished release [optimized] target(s) in 1.24s
     Running `/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-codegen/x86_64-unknown-linux-gnu/release/y test --use-system-gcc --use-backend gcc --out-dir /checkout/obj/build/x86_64-unknown-linux-gnu/stage1-tools/cg_gcc --release --no-default-features --mini-tests --std-tests`
Using system GCC
Using system GCC
[BUILD] example
[AOT] mini_core_hello_world
/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-tools/cg_gcc/mini_core_hello_world
abc
---

10 LL | enum UninhabitedVariantSpace {
11    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
12 
- error: abi: ScalarPair(Initialized { value: Int(I64, false), valid_range: 0..=1 }, Initialized { value: Int(I64, false), valid_range: 0..=18446744073709551615 })
+ error: abi: ScalarPair(Initialized { value: Int(I32, false), valid_range: 0..=1 }, Initialized { value: Int(I64, false), valid_range: 0..=18446744073709551615 })
15    |
15    |
16 LL | enum ScalarPairDifferingSign {

The actual stderr differed from the expected stderr.
Actual stderr saved to /checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/layout/enum/enum.stderr
To update references, rerun the tests and pass the `--bless` flag
To update references, rerun the tests and pass the `--bless` flag
To only update this specific test, also pass `--test-args layout/enum.rs`

error: 1 errors occurred comparing output.
status: exit status: 1
command: RUSTC_ICE="0" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/tests/ui/layout/enum.rs" "-Zthreads=1" "-Zsimulate-remapped-rust-src-base=/rustc/FAKE_PREFIX" "-Ztranslate-remapped-path-to-local-path=no" "-Z" "ignore-directory-in-diagnostics-source-blocks=/cargo" "--sysroot" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2" "--target=i686-unknown-linux-gnu" "--error-format" "json" "--json" "future-incompat" "-Ccodegen-units=1" "-Zui-testing" "-Zdeduplicate-diagnostics=no" "-Zwrite-long-types-to-disk=no" "-Cstrip=debuginfo" "--emit" "metadata" "-C" "prefer-dynamic" "--out-dir" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/layout/enum" "-A" "unused" "-A" "internal_features" "-Crpath" "-Cdebuginfo=0" "-Lnative=/checkout/obj/build/i686-unknown-linux-gnu/native/rust-test-helpers" "-Clinker=x86_64-linux-gnu-gcc" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/layout/enum/auxiliary"
stdout: none
--- stderr -------------------------------
error: align: AbiAndPrefAlign { abi: Align(2 bytes), pref: Align(8 bytes) }
   |
   |
LL | enum UninhabitedVariantAlign { //~ERROR: abi: Align(2 bytes)

error: size: Size(16 bytes)
##[error]  --> /checkout/tests/ui/layout/enum.rs:15:1
   |
   |
LL | enum UninhabitedVariantSpace { //~ERROR: size: Size(16 bytes)


error: abi: ScalarPair(Initialized { value: Int(I32, false), valid_range: 0..=1 }, Initialized { value: Int(I64, false), valid_range: 0..=18446744073709551615 })
   |
   |
LL | enum ScalarPairDifferingSign { //~ERROR: abi: ScalarPair

error: aborting due to 3 previous errors
------------------------------------------

rust-log-analyzer avatar Feb 27 '24 06:02 rust-log-analyzer

:sunny: Try build successful - checks-actions Build commit: 620bc57fd23a71e77bfab2c9db4cb12dff02e970 (620bc57fd23a71e77bfab2c9db4cb12dff02e970)

bors avatar Feb 27 '24 07:02 bors

Queued 620bc57fd23a71e77bfab2c9db4cb12dff02e970 with parent 71ffdf7ff7ac6df5f9f64de7e780b8345797e8a0, future comparison URL. There are currently 2 preceding artifacts in the queue. It will probably take at least ~3.3 hours until the benchmark run finishes.

rust-timer avatar Feb 27 '24 07:02 rust-timer

Finished benchmarking commit (620bc57fd23a71e77bfab2c9db4cb12dff02e970): comparison URL.

Overall result: ❌ regressions - ACTION NEEDED

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

@bors rollup=never @rustbot label: -S-waiting-on-perf +perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
0.7% [0.4%, 0.8%] 5
Regressions ❌
(secondary)
0.4% [0.2%, 1.2%] 27
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-0.8% [-0.8%, -0.8%] 1
All ❌✅ (primary) 0.7% [0.4%, 0.8%] 5

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-3.8% [-3.8%, -3.8%] 1
All ❌✅ (primary) - - 0

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
0.0% [0.0%, 0.0%] 4
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-0.2% [-0.2%, -0.2%] 1
All ❌✅ (primary) 0.0% [0.0%, 0.0%] 4

Bootstrap: 649.3s -> 650.378s (0.17%) Artifact size: 311.08 MiB -> 311.04 MiB (-0.01%)

rust-timer avatar Feb 27 '24 20:02 rust-timer

Hmm, nearly all of the changed benchmarks seem to be noisy, so it's hard to tell if it's real or not...

erikdesjardins avatar Feb 27 '24 23:02 erikdesjardins

looks like noise and it being perf neutral to me

Noratrieb avatar Feb 28 '24 06:02 Noratrieb

In that case...

r? compiler

erikdesjardins avatar Feb 28 '24 22:02 erikdesjardins

The job x86_64-gnu-llvm-16 failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
GITHUB_ACTION=__run_7
GITHUB_ACTIONS=true
GITHUB_ACTION_REF=
GITHUB_ACTION_REPOSITORY=
GITHUB_ACTOR=erikdesjardins
GITHUB_API_URL=https://api.github.com
GITHUB_BASE_REF=master
GITHUB_ENV=/home/runner/work/_temp/_runner_file_commands/set_env_bbe89587-c99c-46ed-b0a4-8d44fcfcb383
GITHUB_EVENT_NAME=pull_request
---
GITHUB_SERVER_URL=https://github.com
GITHUB_SHA=842f471c25d790953192831177b2adaf2290e071
GITHUB_STATE=/home/runner/work/_temp/_runner_file_commands/save_state_bbe89587-c99c-46ed-b0a4-8d44fcfcb383
GITHUB_STEP_SUMMARY=/home/runner/work/_temp/_runner_file_commands/step_summary_bbe89587-c99c-46ed-b0a4-8d44fcfcb383
GITHUB_TRIGGERING_ACTOR=erikdesjardins
GITHUB_WORKFLOW_REF=rust-lang/rust/.github/workflows/ci.yml@refs/pull/121668/merge
GITHUB_WORKFLOW_SHA=842f471c25d790953192831177b2adaf2290e071
GITHUB_WORKSPACE=/home/runner/work/rust/rust
GOROOT_1_19_X64=/opt/hostedtoolcache/go/1.19.13/x64
---
#12 writing image sha256:8500a1bc1d7c5b152683c08ce970913d99d00e6a91853b54ed28b423f4ddc1c4 done
#12 naming to docker.io/library/rust-ci done
#12 DONE 10.1s
##[endgroup]
Setting extra environment values for docker:  --env ENABLE_GCC_CODEGEN=1 --env GCC_EXEC_PREFIX=/usr/lib/gcc/
[CI_JOB_NAME=x86_64-gnu-llvm-16]
##[group]Clock drift check
  local time: Wed Feb 28 22:57:16 UTC 2024
  network time: Wed, 28 Feb 2024 22:57:16 GMT
  network time: Wed, 28 Feb 2024 22:57:16 GMT
##[endgroup]
sccache: Starting the server...
##[group]Configure the build
configure: processing command line
configure: 
configure: build.configure-args := ['--build=x86_64-unknown-linux-gnu', '--llvm-root=/usr/lib/llvm-16', '--enable-llvm-link-shared', '--set', 'rust.thin-lto-import-instr-limit=10', '--set', 'change-id=99999999', '--enable-verbose-configure', '--enable-sccache', '--disable-manage-submodules', '--enable-locked-deps', '--enable-cargo-native-static', '--set', 'rust.codegen-units-std=1', '--set', 'dist.compression-profile=balanced', '--dist-compression-formats=xz', '--set', 'build.optimized-compiler-builtins', '--disable-dist-src', '--release-channel=nightly', '--enable-debug-assertions', '--enable-overflow-checks', '--enable-llvm-assertions', '--set', 'rust.verify-llvm-ir', '--set', 'rust.codegen-backends=llvm,cranelift,gcc', '--set', 'llvm.static-libstdcpp', '--enable-new-symbol-mangling']
configure: target.x86_64-unknown-linux-gnu.llvm-config := /usr/lib/llvm-16/bin/llvm-config
configure: llvm.link-shared     := True
configure: rust.thin-lto-import-instr-limit := 10
configure: change-id            := 99999999
---
##[endgroup]
Testing GCC stage1 (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
   Compiling y v0.1.0 (/checkout/compiler/rustc_codegen_gcc/build_system)
    Finished release [optimized] target(s) in 1.30s
     Running `/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-codegen/x86_64-unknown-linux-gnu/release/y test --use-system-gcc --use-backend gcc --out-dir /checkout/obj/build/x86_64-unknown-linux-gnu/stage1-tools/cg_gcc --release --no-default-features --mini-tests --std-tests`
Using system GCC
Using system GCC
[BUILD] example
[AOT] mini_core_hello_world
/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-tools/cg_gcc/mini_core_hello_world
abc
---

10 LL | enum UninhabitedVariantSpace {
11    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
12 
- error: abi: ScalarPair(Initialized { value: Int(I64, false), valid_range: 0..=1 }, Initialized { value: Int(I64, false), valid_range: 0..=18446744073709551615 })
+ error: abi: ScalarPair(Initialized { value: Int(I32, false), valid_range: 0..=1 }, Initialized { value: Int(I64, false), valid_range: 0..=18446744073709551615 })
15    |
15    |
16 LL | enum ScalarPairDifferingSign {

The actual stderr differed from the expected stderr.
Actual stderr saved to /checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/layout/enum/enum.stderr
To update references, rerun the tests and pass the `--bless` flag
To update references, rerun the tests and pass the `--bless` flag
To only update this specific test, also pass `--test-args layout/enum.rs`

error: 1 errors occurred comparing output.
status: exit status: 1
command: RUSTC_ICE="0" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/tests/ui/layout/enum.rs" "-Zthreads=1" "-Zsimulate-remapped-rust-src-base=/rustc/FAKE_PREFIX" "-Ztranslate-remapped-path-to-local-path=no" "-Z" "ignore-directory-in-diagnostics-source-blocks=/cargo" "--sysroot" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2" "--target=i686-unknown-linux-gnu" "--error-format" "json" "--json" "future-incompat" "-Ccodegen-units=1" "-Zui-testing" "-Zdeduplicate-diagnostics=no" "-Zwrite-long-types-to-disk=no" "-Cstrip=debuginfo" "--emit" "metadata" "-C" "prefer-dynamic" "--out-dir" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/layout/enum" "-A" "unused" "-A" "internal_features" "-Crpath" "-Cdebuginfo=0" "-Lnative=/checkout/obj/build/i686-unknown-linux-gnu/native/rust-test-helpers" "-Clinker=x86_64-linux-gnu-gcc" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/layout/enum/auxiliary"
stdout: none
--- stderr -------------------------------
error: align: AbiAndPrefAlign { abi: Align(2 bytes), pref: Align(8 bytes) }
   |
   |
LL | enum UninhabitedVariantAlign { //~ERROR: abi: Align(2 bytes)

error: size: Size(16 bytes)
##[error]  --> /checkout/tests/ui/layout/enum.rs:15:1
   |
   |
LL | enum UninhabitedVariantSpace { //~ERROR: size: Size(16 bytes)


error: abi: ScalarPair(Initialized { value: Int(I32, false), valid_range: 0..=1 }, Initialized { value: Int(I64, false), valid_range: 0..=18446744073709551615 })
   |
   |
LL | enum ScalarPairDifferingSign { //~ERROR: abi: ScalarPair

error: aborting due to 3 previous errors
------------------------------------------

rust-log-analyzer avatar Feb 28 '24 23:02 rust-log-analyzer

r? compiler

davidtwco avatar Mar 04 '24 11:03 davidtwco

I'm not a good reviewer for this, let's try r? @scottmcm and cc @cuviper for good luck.

nnethercote avatar Mar 05 '24 02:03 nnethercote

Cycle-wise it's showing nothing, so it might just be that instead of one instruction to move the thing it's two now, but that that doesn't actually end up any slower.

scottmcm avatar Mar 05 '24 03:03 scottmcm

Fixes #97540.

This PR fixes any type with a niche, not just Box right? Just double checking that it actually fixes the issue (should be something like Cc rather than Fixes if not)

tgross35 avatar Mar 05 '24 03:03 tgross35

This PR fixes any type with a niche, not just Box right?

The title of https://github.com/rust-lang/rust/issues/97540 is misleading, the niche is not used* in that layout. As such this change doesn't depend on niches at all, it works for any type that gets Scalar(Pointer) layout, i.e. Box, references, raw pointers, etc.

Technically it doesn't fix this for all 16-byte types like the issue title also says, but the primary use case discussed in the issue is for std::io::Result<usize>-like types, which are fixed. And making a heuristic to generalize this to any layout that happens to be 2 x ptr in size is nontrivial without regressing vectorization (as discussed there), so I think it's okay to consider it closed by this change.

* There's no point, since u64/usize uses every bitpattern, so the layout needs to be bigger anyways, and then you might as well use a discriminant (which gives the resulting layout way more niche space) instead of doing something weird like { 0, u64 } | { Box<T>, uninit } just to use the existing niche

erikdesjardins avatar Mar 05 '24 05:03 erikdesjardins

Fixes #97540.

This PR fixes any type with a niche, not just Box right? Just double checking that it actually fixes the issue (should be something like Cc rather than Fixes if not)

The niche is irrelevant here. This optimization triggers for Result<usize, *const ()>, too

oli-obk avatar Mar 05 '24 05:03 oli-obk

Well the codegen test changes LGTM, so coupled with oli's assessment of the layout stuff above, @bors r=scottmcm,oli-obk rollup=never

The pair meaning more instructions sometimes seems fine; the cycles numbers come out even. (Nothing shown as relevant, and if you include non-relevant the primary are ever so slightly green, matched by the secondary being every so slightly red.)

scottmcm avatar Mar 05 '24 07:03 scottmcm

:pushpin: Commit 8e40b17b6bc57e9a905b263b79e254895252ea49 has been approved by scottmcm,oli-obk

It is now in the queue for this repository.

bors avatar Mar 05 '24 07:03 bors

:hourglass: Testing commit 8e40b17b6bc57e9a905b263b79e254895252ea49 with merge ea1e6829881651fa8badf7edaf4bafe402db172b...

bors avatar Mar 06 '24 09:03 bors

The job x86_64-gnu-llvm-17 failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
failures:

---- [codegen] tests/codegen/common_prim_int_ptr.rs stdout ----

error: verification with 'FileCheck' failed
status: exit status: 1
command: "/usr/lib/llvm-17/bin/FileCheck" "--input-file" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/codegen/common_prim_int_ptr/common_prim_int_ptr.ll" "/checkout/tests/codegen/common_prim_int_ptr.rs" "--check-prefix=CHECK" "--check-prefix" "NONMSVC" "--allow-unused-prefixes" "--dump-input-context" "100"
--- stderr -------------------------------
--- stderr -------------------------------
/checkout/tests/codegen/common_prim_int_ptr.rs:32:17: error: CHECK-NEXT: is not on the line after the previous match
 // CHECK-NEXT: ptrtoint
                ^
/checkout/obj/build/x86_64-unknown-linux-gnu/test/codegen/common_prim_int_ptr/common_prim_int_ptr.ll:39:7: note: 'next' match was here
 %1 = ptrtoint ptr %x.1 to i64
      ^
/checkout/obj/build/x86_64-unknown-linux-gnu/test/codegen/common_prim_int_ptr/common_prim_int_ptr.ll:29:7: note: previous match ended here
      ^
      ^
/checkout/obj/build/x86_64-unknown-linux-gnu/test/codegen/common_prim_int_ptr/common_prim_int_ptr.ll:30:1: note: non-matching line after previous match is here
 %0 = icmp eq i64 %x.0, 0
^
/checkout/tests/codegen/common_prim_int_ptr.rs:41:17: error: CHECK-NEXT: is not on the line after the previous match
 // CHECK-NEXT: ret ptr
                ^
/checkout/obj/build/x86_64-unknown-linux-gnu/test/codegen/common_prim_int_ptr/common_prim_int_ptr.ll:55:2: note: 'next' match was here
 ret ptr %x.1
 ^
/checkout/obj/build/x86_64-unknown-linux-gnu/test/codegen/common_prim_int_ptr/common_prim_int_ptr.ll:45:7: note: previous match ended here
      ^
      ^
/checkout/obj/build/x86_64-unknown-linux-gnu/test/codegen/common_prim_int_ptr/common_prim_int_ptr.ll:46:1: note: non-matching line after previous match is here
 %cond.i = icmp eq i64 %x.0, 0

Input file: /checkout/obj/build/x86_64-unknown-linux-gnu/test/codegen/common_prim_int_ptr/common_prim_int_ptr.ll
Check file: /checkout/tests/codegen/common_prim_int_ptr.rs


-dump-input=help explains the following input dump.
Input was:
<<<<<<
<<<<<<
         1: ; ModuleID = 'common_prim_int_ptr.6a84fe6a950ca3c2-cgu.0' 
         2: source_filename = "common_prim_int_ptr.6a84fe6a950ca3c2-cgu.0" 
         3: target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128" 
         4: target triple = "x86_64-unknown-linux-gnu" 
         5:  
         6: @alloc_cb239afd0651186b152d584770c400dc = private unnamed_addr constant <{ [30 x i8] }> <{ [30 x i8] c"assertion failed: self.is_ok()" }>, align 1 
         7: @alloc_ffde696f313e98c15afca937e766111f = private unnamed_addr constant <{ [31 x i8] }> <{ [31 x i8] c"assertion failed: self.is_err()" }>, align 1 
         8: @alloc_04c9a7eb69a8c71295d76dd7a5c10d90 = private unnamed_addr constant <{ [46 x i8] }> <{ [46 x i8] c"/checkout/tests/codegen/common_prim_int_ptr.rs" }>, align 1 
         9: @alloc_64b30cf169fc0d95ede4792d04b46f21 = private unnamed_addr constant <{ ptr, [16 x i8] }> <{ ptr @alloc_04c9a7eb69a8c71295d76dd7a5c10d90, [16 x i8] c".\00\00\00\00\00\00\00\22\00\00\00\07\00\00\00" }>, align 8 
        10: @alloc_9955417c7c1d8a2e6a00ea99071913fd = private unnamed_addr constant <{ ptr, [16 x i8] }> <{ ptr @alloc_04c9a7eb69a8c71295d76dd7a5c10d90, [16 x i8] c".\00\00\00\00\00\00\00*\00\00\00\07\00\00\00" }>, align 8 
        11:  
        12: ; Function Attrs: mustprogress nofree norecurse nosync nounwind nonlazybind willreturn memory(none) uwtable 
        13: define { i64, ptr } @insert_int(i64 noundef %x) unnamed_addr #0 { 
        14: start: 
        15:  %0 = inttoptr i64 %x to ptr 
        16:  %1 = insertvalue { i64, ptr } { i64 0, ptr poison }, ptr %0, 1 
        17:  ret { i64, ptr } %1 
        18: } 
        19:  
        20: ; Function Attrs: mustprogress nofree norecurse nosync nounwind nonlazybind willreturn memory(none) uwtable 
        21: define { i64, ptr } @insert_box(ptr noalias noundef nonnull align 1 %x) unnamed_addr #0 { 
        22: start: 
        23:  %0 = insertvalue { i64, ptr } { i64 1, ptr poison }, ptr %x, 1 
        24:  ret { i64, ptr } %0 
        25: } 
        26:  
        27: ; Function Attrs: nonlazybind uwtable 
        28: define noundef i64 @extract_int(i64 noundef %x.0, ptr noundef %x.1) unnamed_addr #1 personality ptr @rust_eh_personality { 
        29: start: 
        30:  %0 = icmp eq i64 %x.0, 0 
        31:  br i1 %0, label %"_ZN4core6result19Result$LT$T$C$E$GT$16unwrap_unchecked17h80a25c1c2af55878E.exit", label %bb2.i 
        32:  
        33: bb2.i: ; preds = %start 
        34: ; call core::panicking::panic 
        35:  tail call void @_ZN4core9panicking5panic17h300e5d524ef534cdE(ptr noalias noundef nonnull readonly align 1 @alloc_cb239afd0651186b152d584770c400dc, i64 noundef 30, ptr noalias noundef nonnull readonly align 8 dereferenceable(24) @alloc_64b30cf169fc0d95ede4792d04b46f21) #3 
        36:  unreachable 
        37:  
        38: "_ZN4core6result19Result$LT$T$C$E$GT$16unwrap_unchecked17h80a25c1c2af55878E.exit": ; preds = %start 
        39:  %1 = ptrtoint ptr %x.1 to i64 
next:32           !~~~~~~~                  error: match on wrong line
        40:  ret i64 %1 
        41: } 
        42:  
        43: ; Function Attrs: nonlazybind uwtable 
        44: define noundef nonnull align 1 ptr @extract_box(i64 noundef %x.0, ptr noundef readnone returned %x.1) unnamed_addr #1 personality ptr @rust_eh_personality { 
        45: start: 
        46:  %cond.i = icmp eq i64 %x.0, 0 
        47:  br i1 %cond.i, label %bb2.i, label %"_ZN4core6result19Result$LT$T$C$E$GT$20unwrap_err_unchecked17hf2c5fcd639f184f9E.exit" 
        48:  
        49: bb2.i: ; preds = %start 
        50: ; call core::panicking::panic 
        51:  tail call void @_ZN4core9panicking5panic17h300e5d524ef534cdE(ptr noalias noundef nonnull readonly align 1 @alloc_ffde696f313e98c15afca937e766111f, i64 noundef 31, ptr noalias noundef nonnull readonly align 8 dereferenceable(24) @alloc_9955417c7c1d8a2e6a00ea99071913fd) #3 
        52:  unreachable 
        53:  
        54: "_ZN4core6result19Result$LT$T$C$E$GT$20unwrap_err_unchecked17hf2c5fcd639f184f9E.exit": ; preds = %start 
        55:  ret ptr %x.1 
next:41      !~~~~~~       error: match on wrong line
        56: } 
        57:  
        58: ; Function Attrs: nonlazybind uwtable 
        59: declare noundef i32 @rust_eh_personality(i32 noundef, i32 noundef, i64 noundef, ptr noundef, ptr noundef) unnamed_addr #1 
        61: ; core::panicking::panic 
        61: ; core::panicking::panic 
        62: ; Function Attrs: cold noinline noreturn nonlazybind uwtable 
        63: declare void @_ZN4core9panicking5panic17h300e5d524ef534cdE(ptr noalias noundef nonnull readonly align 1, i64 noundef, ptr noalias noundef readonly align 8 dereferenceable(24)) unnamed_addr #2 
        64:  
        65: attributes #0 = { mustprogress nofree norecurse nosync nounwind nonlazybind willreturn memory(none) uwtable "probe-stack"="inline-asm" "target-cpu"="x86-64" } 
        66: attributes #1 = { nonlazybind uwtable "probe-stack"="inline-asm" "target-cpu"="x86-64" } 
        67: attributes #2 = { cold noinline noreturn nonlazybind uwtable "probe-stack"="inline-asm" "target-cpu"="x86-64" } 
        68: attributes #3 = { noreturn } 
        69:  
        70: !llvm.module.flags = !{!0, !1} 
        71: !llvm.ident = !{!2} 
        72:  
        73: !0 = !{i32 8, !"PIC Level", i32 2} 
        74: !1 = !{i32 2, !"RtLibUseGOT", i32 1} 
        75: !2 = !{!"rustc version 1.78.0-nightly (ea1e68298 2024-03-06)"} 
------------------------------------------



rust-log-analyzer avatar Mar 06 '24 10:03 rust-log-analyzer

:broken_heart: Test failed - checks-actions

bors avatar Mar 06 '24 10:03 bors

The job x86_64-gnu-llvm-17 failed!

It seems LLVM 17 doesn't optimize out the unwrap_unchecked. Loosened the check lines since we don't care about the precise instruction sequence.

erikdesjardins avatar Mar 07 '24 00:03 erikdesjardins

Ping @scottmcm, I made some changes to the codegen tests so they also pass on LLVM 17. Let me know if you'd prefer to revert the last commit and use min-llvm-version: 18 instead.

erikdesjardins avatar Mar 12 '24 21:03 erikdesjardins

Yeah, something like that works.

@rustbot ready

erikdesjardins avatar Mar 13 '24 05:03 erikdesjardins

Oh, right, scalar pair, so clearly more than one parameter 🤦

Test updates look good: @bors r=scottmcm,oli-obk rollup=never

scottmcm avatar Mar 13 '24 15:03 scottmcm

:pushpin: Commit 9f55200a42f599549fee0f003b27888d03604861 has been approved by scottmcm,oli-obk

It is now in the queue for this repository.

bors avatar Mar 13 '24 15:03 bors

:hourglass: Testing commit 9f55200a42f599549fee0f003b27888d03604861 with merge 3cbb93223f33024db464a4df27a13c7cce870173...

bors avatar Mar 13 '24 15:03 bors

:sunny: Test successful - checks-actions Approved by: scottmcm,oli-obk Pushing 3cbb93223f33024db464a4df27a13c7cce870173 to master...

bors avatar Mar 13 '24 17:03 bors

Finished benchmarking commit (3cbb93223f33024db464a4df27a13c7cce870173): comparison URL.

Overall result: ❌✅ regressions and improvements - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
1.1% [1.1%, 1.1%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-3.5% [-3.5%, -3.5%] 1
All ❌✅ (primary) - - 0

Max RSS (memory usage)

This benchmark run did not return any relevant results for this metric.

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-3.0% [-3.0%, -3.0%] 1
All ❌✅ (primary) - - 0

Binary size

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
0.0% [0.0%, 0.2%] 5
Regressions ❌
(secondary)
1.0% [1.0%, 1.3%] 28
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-0.2% [-0.2%, -0.2%] 1
All ❌✅ (primary) 0.0% [0.0%, 0.2%] 5

Bootstrap: 675.199s -> 675.071s (-0.02%) Artifact size: 310.80 MiB -> 310.73 MiB (-0.02%)

rust-timer avatar Mar 13 '24 18:03 rust-timer