Try enabling precondition checks on ptr::{read,write} #129498

saethlin · 2024-08-24T04:04:47Z

The overhead here is probably too much, but let's have the measurement anyway.

This will fail at least one codegen test.

r? @ghost

Ralf says: If we keep this attribute, the docs on the intrinsic should be updated.

saethlin · 2024-08-24T04:04:54Z

@bors try @rust-timer queue

bors · 2024-08-24T04:06:05Z

⌛ Trying commit 0a65e30 with merge aadda4d...

…, r=<try> Try enabling precondition checks on ptr::{read,write} The overhead here is probably too much, but let's have the measurement anyway. This will fail at least one codegen test. r? `@ghost`

bors · 2024-08-24T05:50:51Z

☀️ Try build successful - checks-actions
Build commit: aadda4d (aadda4d695969f6a5337d0bf19f1e6572bd67922)

rust-timer · 2024-08-24T07:07:18Z

Finished benchmarking commit (aadda4d): comparison URL.

Overall result: ❌✅ regressions and improvements - 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.2%, 1.9%]	52
Regressions ❌ (secondary)	2.0%	[0.2%, 6.3%]	32
Improvements ✅ (primary)	-0.6%	[-0.9%, -0.4%]	3
Improvements ✅ (secondary)	-1.3%	[-1.3%, -1.3%]	1
All ❌✅ (primary)	0.6%	[-0.9%, 1.9%]	55

Max RSS (memory usage)

Results (primary 0.3%)

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)	3.1%	[0.6%, 5.0%]	4
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-3.3%	[-3.7%, -2.8%]	3
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.3%	[-3.7%, 5.0%]	7

Cycles

Results (primary 1.5%, secondary 3.3%)

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)	1.5%	[0.7%, 2.1%]	6
Regressions ❌ (secondary)	3.3%	[2.1%, 5.3%]	8
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	1.5%	[0.7%, 2.1%]	6

Binary size

Results (primary 0.6%, secondary 0.6%)

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.7%	[0.0%, 6.1%]	77
Regressions ❌ (secondary)	3.2%	[1.1%, 3.7%]	12
Improvements ✅ (primary)	-0.3%	[-0.5%, -0.1%]	11
Improvements ✅ (secondary)	-0.2%	[-3.1%, -0.0%]	43
All ❌✅ (primary)	0.6%	[-0.5%, 6.1%]	88

Bootstrap: 751.449s -> 756.74s (0.70%)
Artifact size: 339.00 MiB -> 338.92 MiB (-0.02%)

saethlin · 2024-08-28T00:44:37Z

@bors try @rust-timer queue

…, r=<try> Try enabling precondition checks on ptr::{read,write} The overhead here is probably too much, but let's have the measurement anyway. This will fail at least one codegen test. r? `@ghost`

bors · 2024-08-28T00:45:48Z

⌛ Trying commit 3b756c6 with merge 4091821...

bors · 2024-08-28T02:29:28Z

☀️ Try build successful - checks-actions
Build commit: 4091821 (40918219c81cfdde936a6a9166876866d77b7d1f)

rust-timer · 2024-08-28T04:40:37Z

Finished benchmarking commit (4091821): comparison URL.

Overall result: ❌✅ regressions and improvements - 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.5%	[0.2%, 1.6%]	32
Regressions ❌ (secondary)	0.9%	[0.2%, 2.4%]	7
Improvements ✅ (primary)	-0.5%	[-0.5%, -0.5%]	2
Improvements ✅ (secondary)	-1.6%	[-1.6%, -1.6%]	1
All ❌✅ (primary)	0.5%	[-0.5%, 1.6%]	34

Max RSS (memory usage)

Results (primary 1.1%, secondary -2.3%)

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)	9.5%	[5.0%, 14.0%]	2
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-3.2%	[-4.1%, -2.4%]	4
Improvements ✅ (secondary)	-2.3%	[-3.4%, -1.7%]	6
All ❌✅ (primary)	1.1%	[-4.1%, 14.0%]	6

Cycles

Results (primary 1.6%, secondary 1.7%)

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)	1.6%	[1.1%, 2.4%]	7
Regressions ❌ (secondary)	2.5%	[2.0%, 2.9%]	4
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-1.5%	[-1.5%, -1.5%]	1
All ❌✅ (primary)	1.6%	[1.1%, 2.4%]	7

Binary size

Results (primary 0.4%, secondary 0.6%)

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.6%	[0.0%, 6.2%]	62
Regressions ❌ (secondary)	1.2%	[0.3%, 3.8%]	14
Improvements ✅ (primary)	-0.2%	[-0.4%, -0.0%]	15
Improvements ✅ (secondary)	-0.4%	[-3.1%, -0.0%]	9
All ❌✅ (primary)	0.4%	[-0.4%, 6.2%]	77

Bootstrap: 752.135s -> 753.574s (0.19%)
Artifact size: 338.76 MiB -> 338.74 MiB (-0.01%)

RalfJung · 2024-08-28T06:12:51Z

compiler/rustc_mir_transform/src/instsimplify.rs

        };
        let preserve_ub_checks =
            attr::contains_name(tcx.hir().krate_attrs(), sym::rustc_preserve_ub_checks);
+        let remove_ub_checks = tcx.has_attr(def_id, sym::rustc_no_ubchecks);


If we keep this attribute, the docs on the intrinsic should be updated.

bors · 2024-08-30T05:54:26Z

☀️ Try build successful - checks-actions
Build commit: 89b5c9e (89b5c9e1eb61f1414d7a3fa66591027d0a3a5725)

RalfJung · 2024-08-30T06:31:41Z

compiler/rustc_mir_transform/src/instsimplify.rs

@@ -45,7 +45,12 @@ impl<'tcx> MirPass<'tcx> for InstSimplify {
        };
        let preserve_ub_checks =
            attr::contains_name(tcx.hir().krate_attrs(), sym::rustc_preserve_ub_checks);
-        let remove_ub_checks = tcx.has_attr(def_id, sym::rustc_no_ubchecks);
+        // FIXME(async_closures) tcx.has_attr on async closures seems to ICE. Not sure why.
+        let remove_ub_checks = if tcx.is_coroutine(def_id) {


Cc @compiler-errors

rust-timer · 2024-08-30T07:11:29Z

Finished benchmarking commit (89b5c9e): comparison URL.

Overall result: ❌✅ regressions and improvements - 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.2%, 3.2%]	27
Regressions ❌ (secondary)	0.4%	[0.3%, 0.7%]	14
Improvements ✅ (primary)	-0.4%	[-0.4%, -0.3%]	3
Improvements ✅ (secondary)	-1.2%	[-2.9%, -0.4%]	3
All ❌✅ (primary)	0.6%	[-0.4%, 3.2%]	30

Max RSS (memory usage)

Results (primary -0.0%, secondary -0.5%)

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)	1.4%	[0.4%, 8.5%]	34
Regressions ❌ (secondary)	0.7%	[0.4%, 1.3%]	29
Improvements ✅ (primary)	-1.0%	[-4.8%, -0.4%]	49
Improvements ✅ (secondary)	-1.0%	[-3.9%, -0.4%]	76
All ❌✅ (primary)	-0.0%	[-4.8%, 8.5%]	83

Cycles

Results (primary 0.4%, secondary 0.5%)

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.8%	[0.4%, 5.5%]	50
Regressions ❌ (secondary)	1.2%	[0.4%, 5.4%]	85
Improvements ✅ (primary)	-0.9%	[-3.0%, -0.4%]	15
Improvements ✅ (secondary)	-1.0%	[-3.2%, -0.4%]	38
All ❌✅ (primary)	0.4%	[-3.0%, 5.5%]	65

Binary size

Results (primary 0.8%)

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.8%	[0.0%, 6.0%]	31
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.1%	[-0.2%, -0.1%]	2
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.8%	[-0.2%, 6.0%]	33

Bootstrap: 788.226s -> 791.844s (0.46%)
Artifact size: 338.74 MiB -> 338.74 MiB (0.00%)

saethlin · 2024-09-03T23:28:10Z

@bors try @rust-timer queue

bors · 2024-09-03T23:29:21Z

⌛ Trying commit 2986cc0 with merge 498c217...

…, r=<try> Try enabling precondition checks on ptr::{read,write} The overhead here is probably too much, but let's have the measurement anyway. This will fail at least one codegen test. r? `@ghost` - [ ] Ralf says: If we keep this attribute, the docs on the intrinsic should be updated.

bors · 2024-09-04T01:13:54Z

☀️ Try build successful - checks-actions
Build commit: 498c217 (498c21794ec20de1231252b78a9517c130cd0d9c)

rust-timer · 2024-09-04T02:28:45Z

Finished benchmarking commit (498c217): 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.2%, 1.8%]	12
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.3%	[-0.3%, -0.3%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.6%	[-0.3%, 1.8%]	13

Max RSS (memory usage)

Results (primary 2.8%, secondary -3.9%)

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)	6.3%	[3.5%, 10.5%]	5
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-3.1%	[-3.4%, -2.7%]	3
Improvements ✅ (secondary)	-3.9%	[-5.8%, -2.1%]	2
All ❌✅ (primary)	2.8%	[-3.4%, 10.5%]	8

Cycles

Results (primary 1.4%)

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)	1.4%	[1.3%, 1.5%]	3
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	1.4%	[1.3%, 1.5%]	3

Binary size

Results (primary 0.2%)

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.3%	[0.0%, 5.6%]	49
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.2%	[-0.6%, -0.1%]	7
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.2%	[-0.6%, 5.6%]	56

Bootstrap: 750.76s -> 748.129s (-0.35%)
Artifact size: 338.30 MiB -> 338.29 MiB (-0.01%)

saethlin · 2024-09-04T02:53:40Z

@bors try @rust-timer queue

…, r=<try> Try enabling precondition checks on ptr::{read,write} The overhead here is probably too much, but let's have the measurement anyway. This will fail at least one codegen test. r? `@ghost` - [ ] Ralf says: If we keep this attribute, the docs on the intrinsic should be updated.

bors · 2024-09-04T02:54:51Z

⌛ Trying commit f9b6f51 with merge bbda6ab...

bors · 2024-09-04T04:47:30Z

☀️ Try build successful - checks-actions
Build commit: bbda6ab (bbda6ab2cc1893c2fe3bf8d76027cea1c5c14416)

rust-timer · 2024-09-04T06:02:43Z

Finished benchmarking commit (bbda6ab): comparison URL.

Overall result: ❌✅ regressions and improvements - 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.5%	[0.2%, 1.0%]	15
Regressions ❌ (secondary)	0.5%	[0.3%, 0.6%]	3
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-2.6%	[-2.6%, -2.6%]	1
All ❌✅ (primary)	0.5%	[0.2%, 1.0%]	15

Max RSS (memory usage)

Results (primary 0.8%, secondary -2.6%)

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)	4.1%	[2.3%, 5.5%]	4
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-3.7%	[-4.2%, -3.0%]	3
Improvements ✅ (secondary)	-2.6%	[-2.6%, -2.6%]	1
All ❌✅ (primary)	0.8%	[-4.2%, 5.5%]	7

Cycles

Results (primary 1.5%)

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)	1.5%	[0.9%, 2.0%]	2
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	1.5%	[0.9%, 2.0%]	2

Binary size

Results (primary 0.2%, secondary 0.2%)

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.3%	[0.0%, 1.1%]	61
Regressions ❌ (secondary)	0.3%	[0.1%, 0.4%]	11
Improvements ✅ (primary)	-0.2%	[-0.3%, -0.1%]	6
Improvements ✅ (secondary)	-0.2%	[-0.2%, -0.2%]	3
All ❌✅ (primary)	0.2%	[-0.3%, 1.1%]	67

Bootstrap: 750.76s -> 751.514s (0.10%)
Artifact size: 338.30 MiB -> 338.40 MiB (0.03%)

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Aug 24, 2024

This comment has been minimized.

Sign in to view

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Aug 24, 2024

This comment has been minimized.

Sign in to view

rustbot added perf-regression Performance regression. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Aug 24, 2024

workingjubilee mentioned this pull request Aug 24, 2024

Pass fmt::Arguments by reference to PanicInfo and PanicMessage #129491

Merged

saethlin added S-experimental Status: Ongoing experiment that does not require reviewing and won't be merged in its current state. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 24, 2024

saethlin force-pushed the ptr-read-write-precondition branch from 0a65e30 to 3b756c6 Compare August 28, 2024 00:42

This comment has been minimized.

Sign in to view

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Aug 28, 2024

saethlin mentioned this pull request Aug 28, 2024

Disabling particular unsafe precondition checks under -Cdebug-assertions=y #120969

Open

This comment has been minimized.

Sign in to view

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Aug 28, 2024

RalfJung reviewed Aug 28, 2024

View reviewed changes

ojeda mentioned this pull request Aug 28, 2024

Rust wanted features Rust-for-Linux/linux#354

Open

36 tasks

saethlin force-pushed the ptr-read-write-precondition branch from 3b756c6 to d527afc Compare August 29, 2024 22:32

This comment has been minimized.

Sign in to view

RalfJung reviewed Aug 30, 2024

View reviewed changes

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Aug 30, 2024

saethlin added 5 commits September 1, 2024 16:54

Try enabling precondition checks on ptr::{read,write}

bde90b6

Add #[rustc_no_ubchecks]

3130990

paper over the ICE

a1e9fcb

Another

deb9d38

cfg out the check in write

2986cc0

saethlin force-pushed the ptr-read-write-precondition branch from e001e0b to 2986cc0 Compare September 1, 2024 21:17

This comment has been minimized.

Sign in to view

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Sep 3, 2024

This comment has been minimized.

Sign in to view

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Sep 4, 2024

cfg out the check in read

f9b6f51

This comment has been minimized.

Sign in to view

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Sep 4, 2024

This comment has been minimized.

Sign in to view

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Sep 4, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Try enabling precondition checks on ptr::{read,write} #129498

Try enabling precondition checks on ptr::{read,write} #129498

saethlin commented Aug 24, 2024 •

edited

Loading

saethlin commented Aug 24, 2024

This comment has been minimized.

bors commented Aug 24, 2024

bors commented Aug 24, 2024

This comment has been minimized.

rust-timer commented Aug 24, 2024

saethlin commented Aug 28, 2024

This comment has been minimized.

bors commented Aug 28, 2024

This comment has been minimized.

bors commented Aug 28, 2024

This comment has been minimized.

rust-timer commented Aug 28, 2024

RalfJung Aug 28, 2024

bors commented Aug 30, 2024

This comment has been minimized.

RalfJung Aug 30, 2024

rust-timer commented Aug 30, 2024

saethlin commented Sep 3, 2024

This comment has been minimized.

bors commented Sep 3, 2024

bors commented Sep 4, 2024

This comment has been minimized.

rust-timer commented Sep 4, 2024

saethlin commented Sep 4, 2024

This comment has been minimized.

bors commented Sep 4, 2024

bors commented Sep 4, 2024

This comment has been minimized.

rust-timer commented Sep 4, 2024

Try enabling precondition checks on ptr::{read,write} #129498

Are you sure you want to change the base?

Try enabling precondition checks on ptr::{read,write} #129498

Conversation

saethlin commented Aug 24, 2024 • edited Loading

saethlin commented Aug 24, 2024

This comment has been minimized.

bors commented Aug 24, 2024

bors commented Aug 24, 2024

This comment has been minimized.

rust-timer commented Aug 24, 2024

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

saethlin commented Aug 28, 2024

This comment has been minimized.

bors commented Aug 28, 2024

This comment has been minimized.

bors commented Aug 28, 2024

This comment has been minimized.

rust-timer commented Aug 28, 2024

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

Choose a reason for hiding this comment

bors commented Aug 30, 2024

This comment has been minimized.

Choose a reason for hiding this comment

rust-timer commented Aug 30, 2024

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

saethlin commented Sep 3, 2024

This comment has been minimized.

bors commented Sep 3, 2024

bors commented Sep 4, 2024

This comment has been minimized.

rust-timer commented Sep 4, 2024

Overall result: ❌ regressions - ACTION NEEDED

saethlin commented Sep 4, 2024

This comment has been minimized.

bors commented Sep 4, 2024

bors commented Sep 4, 2024

This comment has been minimized.

rust-timer commented Sep 4, 2024

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

saethlin commented Aug 24, 2024 •

edited

Loading