5

Add 0..=isize::MAX range metadata to size loads from vtables by erikdesjardins ·...

 1 year ago
source link: https://github.com/rust-lang/rust/pull/105446
Go to the source link to view the article. You can view the picture content, updated content and better typesetting reading experience. If the link is broken, please click the button below to view the snapshot at that time.
neoserver,ios ssh client

Add 0..=isize::MAX range metadata to size loads from vtables #105446

Conversation

Contributor

@erikdesjardins erikdesjardins commented Dec 8, 2022

This is the (much belated) size counterpart to #91569.

Inspired by https://rust-lang.zulipchat.com/#narrow/stream/187780-t-compiler.2Fwg-llvm/topic/Range.20metadata.20for.20.60size_of_val.60.20and.20other.20isize.3A.3AMAX.20limits. This could help optimize layout computations based on the size of a dyn trait. Though, admittedly, adding this to vtables wouldn't be as beneficial as adding it to slice len, which is used much more often.

Miri detects this UB already:

(In fact Miri goes further, assuming a 48-bit address space on 64-bit platforms, but I don't think we can assume that in an optimization.)
scottmcm and mati865 reacted with heart emoji

Collaborator

rustbot commented Dec 8, 2022

r? @petrochenkov

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

rustbot

added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

labels

Dec 8, 2022

Contributor

nikic commented Dec 8, 2022

This comment has been minimized.

rustbot

added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label

Dec 8, 2022

Contributor

bors commented Dec 8, 2022

hourglass Trying commit a99e97a with merge 1e9be93...

Contributor

bors commented Dec 8, 2022

sunny Try build successful - checks-actions
Build commit: 1e9be93 (1e9be934e289bcfe6465db23c6e5cdbd94078d7c)

This comment has been minimized.

Collaborator

rust-timer commented Dec 8, 2022

Finished benchmarking commit (1e9be93): comparison URL.

Overall result: no relevant changes - no 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.

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

Instruction count

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

Max RSS (memory usage)

Results

Cycles

Results

rustbot

removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label

Dec 8, 2022

Contributor

petrochenkov commented Dec 18, 2022

r? @nikic

Contributor

nikic commented Dec 18, 2022

@bors r+

Contributor

bors commented Dec 18, 2022

pushpin Commit e01d944 has been approved by nikic

It is now in the queue for this repository.

bors

added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.

and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.

labels

Dec 18, 2022

Contributor

bors commented Dec 18, 2022

hourglass Testing commit e01d944 with merge 2b094b1...

Contributor

bors commented Dec 19, 2022

sunny Test successful - checks-actions
Approved by: nikic
Pushing 2b094b1 to master...

bors

added the merged-by-bors This PR was explicitly merged by bors label

Dec 19, 2022

bors

merged commit 2b094b1 into

rust-lang:master

Dec 19, 2022

11 checks passed

Collaborator

rust-timer commented Dec 19, 2022

Finished benchmarking commit (2b094b1): comparison URL.

Overall result: white_check_mark 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 x
(primary)
- - 0
Regressions x
(secondary)
- - 0
Improvements white_check_mark
(primary)
- - 0
Improvements white_check_mark
(secondary)
-1.1% [-1.1%, -1.1%] 1
All xwhite_check_mark (primary) - - 0

Max RSS (memory usage)

Results

Cycles

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

scottmcm

Assignees

nikic

Labels
merged-by-bors This PR was explicitly merged by bors S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects

None yet

Milestone

1.68.0

Development

Successfully merging this pull request may close these issues.

None yet

7 participants

About Joyk


Aggregate valuable and interesting links.
Joyk means Joy of geeK