Skip to content

Unstably allow assume intrinsic in const contexts - #76973

Merged
bors merged 3 commits into
rust-lang:masterfrom
tesuji:unstably-const-assume
Sep 25, 2020
Merged

Unstably allow assume intrinsic in const contexts#76973
bors merged 3 commits into
rust-lang:masterfrom
tesuji:unstably-const-assume

Conversation

@tesuji

@tesujitesuji commented Sep 20, 2020•
edited
Loading

Copy link
Copy Markdown
Contributor

Not sure much about this usage because there are concerns
about blocking optimization and slowing down LLVM when using assme intrinsic
in inline functions.
But since Oli suggested in #76960 (comment),
here we are.

@rust-highfiverust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 20, 2020
oli-obk
oli-obk reviewed Sep 20, 2020
View reviewed changes
Comment threadsrc/test/ui/consts/const-eval/const_assume.rs Outdated
@tesuji
tesujiforce-pushed the unstably-const-assume branch from 7d25016 to 4bb75ddCompareSeptember 20, 2020 13:57
@oli-obkoli-obk mentioned this pull request Sep 20, 2020
Closed
3 tasks
@tesuji
tesujiforce-pushed the unstably-const-assume branch from 4bb75dd to a3f87fcCompareSeptember 20, 2020 14:01
RalfJung
RalfJung reviewed Sep 20, 2020
View reviewed changes
Comment threadcompiler/rustc_mir/src/interpret/intrinsics.rs Outdated
@tesuji
tesujiforce-pushed the unstably-const-assume branch from a3f87fc to c2390c9CompareSeptember 20, 2020 14:03
RalfJung
RalfJung reviewed Sep 20, 2020
View reviewed changes
Comment threadsrc/test/ui/consts/const-eval/const_assume.rs Outdated

@RalfJungRalfJungSep 20, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this the kind of test that should be made a unit test instead (#76268)?

@tesujitesujiSep 20, 2020•
edited
Loading

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I only saw const_slice_from_raw_parts and const_raw_ptr_deref in library/core/tests/lib.rs.
Not really sure about this.

@matkladmatkladSep 24, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I think this should be a unit-test.

@tesujitesujiSep 24, 2020•
edited
Loading

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's kind of weird since most intrinsics not tested in library/core/tests.
Anyway, I moved it.

oli-obk
oli-obk reviewed Sep 20, 2020
View reviewed changes
Comment threadlibrary/core/src/num/nonzero.rs Outdated
@tesuji

tesuji commented Sep 20, 2020

Copy link
Copy Markdown
ContributorAuthor

Not sure the block optimization concerns about adding assume to NonZero*::get still hold: #54995 (comment)

@tesuji

tesuji commented Sep 20, 2020•
edited
Loading

Copy link
Copy Markdown
ContributorAuthor

Because the above concern isn't resolved, I will move the assume part in NonZero*::get to another PR
or just drop it.

@oli-obk

oli-obk commented Sep 20, 2020

Copy link
Copy Markdown
Contributor

Yea they probably do :( I don't think there's any point in summoning nagisa or hanna, I don't think anything changed over the last two years that woudl affect this.

@tesuji
tesujiforce-pushed the unstably-const-assume branch from c2390c9 to 4387480CompareSeptember 20, 2020 14:17
@tesujitesuji mentioned this pull request Sep 20, 2020
Merged
@oli-obk

oli-obk commented Sep 24, 2020

Copy link
Copy Markdown
Contributor

@bors r+

@bors

bors commented Sep 24, 2020

Copy link
Copy Markdown
Collaborator

📌 Commit 382d724 has been approved by oli-obk

@borsbors 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 Sep 24, 2020
@RalfJung

RalfJung commented Sep 25, 2020

Copy link
Copy Markdown
Member

@bors rollup

RalfJung added a commit to RalfJung/rust that referenced this pull request Sep 25, 2020
@RalfJung
Rollup merge ofrust-lang#76973- lzutao:unstably-const-assume, r=oli…
287fdff
…-obk
Unstably allow assume intrinsic in const contexts
Not sure much about this usage because there are concerns
about [blocking optimization][1] and [slowing down LLVM][2] when using `assme` intrinsic
in inline functions.
But since Oli suggested in rust-lang#76960 (comment),
here we are.
[1]: rust-lang#54995 (comment)
[2]: rust-lang#49572 (comment)
@RalfJungRalfJung mentioned this pull request Sep 25, 2020
Closed
RalfJung added a commit to RalfJung/rust that referenced this pull request Sep 25, 2020
@RalfJung
Rollup merge ofrust-lang#76973- lzutao:unstably-const-assume, r=oli…
c822e91
…-obk
Unstably allow assume intrinsic in const contexts
Not sure much about this usage because there are concerns
about [blocking optimization][1] and [slowing down LLVM][2] when using `assme` intrinsic
in inline functions.
But since Oli suggested in rust-lang#76960 (comment),
here we are.
[1]: rust-lang#54995 (comment)
[2]: rust-lang#49572 (comment)
@RalfJungRalfJung mentioned this pull request Sep 25, 2020
Closed
@197g197g mentioned this pull request Sep 25, 2020
Merged
@jonas-schievinkjonas-schievink mentioned this pull request Sep 25, 2020
Closed
This was referenced Sep 25, 2020
Closed
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 25, 2020
@bors
Auto merge ofrust-lang#77198- jonas-schievink:rollup-i59i41h, r=jon…
c6e4db6
…as-schievink
Rollup of 15 pull requests
Successful merges:
- rust-lang#76932 (Relax promises about condition variable.)
- rust-lang#76973 (Unstably allow assume intrinsic in const contexts)
- rust-lang#77005 (BtreeMap: refactoring around edges)
- rust-lang#77066 (Fix dest prop miscompilation around references)
- rust-lang#77073 (dead_code: look at trait impls even if they don't contain items)
- rust-lang#77086 (Include libunwind in the rust-src component.)
- rust-lang#77097 (Make [].as_[mut_]ptr_range() (unstably) const.)
- rust-lang#77106 (clarify that `changelog-seen = 1` goes to the beginning of config.toml)
- rust-lang#77120 (Add `--keep-stage-std` to `x.py` for keeping only standard library artifacts)
- rust-lang#77126 (Invalidate local LLVM cache less often)
- rust-lang#77146 (Install std for non-host targets)
- rust-lang#77155 (remove enum name from ImplSource variants)
- rust-lang#77176 (Removing erroneous semicolon in transmute documentation)
- rust-lang#77183 (Allow multiple allow_internal_unstable attributes)
- rust-lang#77189 (Remove extra space from vec drawing)
Failed merges:
r? `@ghost`
@bors
bors merged commit 1b8c939 into rust-lang:masterSep 25, 2020
@rustbotrustbot added this to the 1.48.0 milestone Sep 25, 2020
@tesuji
tesuji deleted the unstably-const-assume branch September 26, 2020 03:05
bors added a commit to rust-lang/miri that referenced this pull request Sep 27, 2020
@bors
Auto merge of#1555- lzutao:upstream-assume-const, r=RalfJung
aa832c1
Remove assume intrinsic from EvalContextExt
Waiting for rust-lang/rust#76973 merged.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@RalfJungRalfJungRalfJung left review comments
@oli-obkoli-obkoli-obk left review comments
+1 more reviewer
@matkladmatkladmatklad left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@oli-obkoli-obk

Labels

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

Projects

None yet

Milestone

1.48.0

Development

Successfully merging this pull request may close these issues.

7 participants

@tesuji@oli-obk@bors@RalfJung@matklad@rust-highfive@rustbot