Add `CastKind::Transmute` to MIR by scottmcm · Pull Request #108442 · rust-lang/rust

scottmcm · 2023-02-25T02:36:10Z

~~Nothing actually produces it in this commit, so I don't know how to test it, but it also means it shouldn't be possible for it to break anything.~~

Includes lowering transmute calls to it, so it's used.

Zulip Conversation: https://rust-lang.zulipchat.com/#narrow/stream/189540-t-compiler.2Fwg-mir-opt/topic/Good.20first.20isssue/near/321849610

rustbot · 2023-02-25T02:36:15Z

Failed to set assignee to JakobDegen: invalid assignee

Note: Only org members, users with write permissions, or people who have commented on the PR may be assigned.

rustbot · 2023-02-25T02:36:18Z

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

This PR changes MIR

cc @oli-obk, @RalfJung, @JakobDegen, @davidtwco, @celinval, @vakaras

Some changes occurred to the CTFE / Miri engine

cc @rust-lang/miri

rustbot · 2023-02-25T02:47:11Z

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

oli-obk · 2023-02-25T06:32:41Z

To test it you could make the intrinsic mir pass convert the transmute function calls into an assignment of the new rvalue. That should allow removing all the intrinsic logic for transmute from the various backends

rustbot · 2023-02-25T23:32:45Z

The Miri subtree was changed

cc @rust-lang/miri

JakobDegen · 2023-02-27T00:46:11Z

+ self.fail(location, format!("Source and destination layouts have different sizes: {src_layout:?} vs {dst_layout:?}"));
+ }
+ }
+ }


Hmm, thinking about it more, I actually don't think we should have any check like this. Transmutes are presumably things that optimizations want to introduce, and making these kinds of cases not well-formed just feels like a footgun for those opts.
It's probably fine to keep this in for now, but can you include a comment that indicates that we can and should relax this restriction if some opts run into the need for it?

Yeah, relaxing it in future sounds reasonable. I've added the comment, since relaxing it will probably need a bunch of intrinsics::mir! tests around how the various consumers would handle such a thing, which I'd rather not do right now.

scottmcm · 2023-02-27T18:13:56Z

@oli-obk I see you resolved some comment threads here. Anything else you need from me before this is good to go?

(If it's just that you need time to thoroughly review it that's totally ok!)

oli-obk · 2023-02-28T08:20:41Z

(If it's just that you need time to thoroughly review it that's totally ok!)

yea, just needed to get to a PC to be able to review the diff properly and look at code that isn't nearby your changes.

@bors r+

bors · 2023-02-28T08:20:43Z

📌 Commit 6f49f85 has been approved by oli-obk

It is now in the queue for this repository.

Detect uninhabited types early in const eval r? `@RalfJung` implements rust-lang/rust#108442 (comment) this is a breaking change, as some UB during const eval is now detected instead of silently being ignored. Users can see this and other UB that may cause future breakage with `-Zextra-const-ub-checks` or just by running miri on their code, which sets that flag by default.

oli-obk · 2023-03-23T07:41:39Z

+ let size_in_bytes = src.layout.size.bytes();
+ if size_in_bytes == 0 {


This could also be
Suggested change
let size_in_bytes = src.layout.size.bytes();
if size_in_bytes == 0{
if src.layout.is_zst(){

oli-obk · 2023-03-23T07:45:49Z

+// These cases are statically rejected by `mem::transmute`, so we need custom
+// MIR to be able to get to constant evaluation.


✨ this is an awesome test

Glad you like it! I was really impressed with the custom mir stuff; way easier to use and update than I'd worried it would be.

oli-obk · 2023-03-23T07:46:36Z

@bors r+

bors · 2023-03-23T07:46:38Z

📌 Commit 64cce5f has been approved by oli-obk

It is now in the queue for this repository.

bors · 2023-03-23T17:56:07Z

⌛ Testing commit 64cce5f with merge cd1c3b968ac7aec48620234dd7481cdbd657295d...

bors · 2023-03-23T18:21:53Z

💔 Test failed - checks-actions

rust-log-analyzer · 2023-03-23T18:23:06Z

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

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

---- [ui] tests/ui/union/union-drop-assign.rs stdout ----
error: test run failed!
status: exit status: 101
command: RUST_TEST_THREADS="16" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/remote-test-client" "run" "0" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/union/union-drop-assign/a"
uploaded "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/union/union-drop-assign/a", waiting for result
------------------------------------------
--- stderr -------------------------------
thread 'main' panicked at 'client.read_exact(&mut header) failed with Connection reset by peer (os error 104)', src/tools/remote-test-client/src/main.rs:310:9

oli-obk · 2023-03-23T18:25:00Z

@bors retry bootstrap self test network failure

bors · 2023-03-23T18:43:07Z

⌛ Testing commit 64cce5f with merge e216300...

bors · 2023-03-23T21:16:48Z

☀️ Test successful - checks-actions
Approved by: oli-obk
Pushing e216300 to master...

rust-timer · 2023-03-23T23:14:52Z

Finished benchmarking commit (e216300): comparison URL.

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please open an issue or create a new PR that fixes the regressions, add a comment linking to the newly created issue or PR, and then add the perf-regression-triaged label to this PR.

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

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)	0.7%	[0.3%, 1.5%]	4
Improvements ✅ (primary)	-1.6%	[-1.6%, -1.6%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-1.6%	[-1.6%, -1.6%]	1

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)	3.0%	[2.9%, 3.1%]	2
Regressions ❌ (secondary)	1.2%	[0.9%, 1.5%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.9%	[-2.1%, -0.4%]	10
All ❌✅ (primary)	3.0%	[2.9%, 3.1%]	2

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.8%	[0.4%, 1.2%]	4
Improvements ✅ (primary)	-1.4%	[-1.4%, -1.4%]	1
Improvements ✅ (secondary)	-0.7%	[-1.3%, -0.5%]	5
All ❌✅ (primary)	-1.4%	[-1.4%, -1.4%]	1

rylev · 2023-03-28T14:33:43Z

The regressions are small enough that I don't think this is worth investigating.

@rustbot label: +perf-regression-triaged

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 Feb 25, 2023

scottmcm force-pushed the mir-transmute branch from fd0978a to c6bce48Compare February 25, 2023 02:47

scottmcm assigned oli-obk Feb 25, 2023

JakobDegen reviewed Feb 25, 2023
View reviewed changes

Comment threadsrc/tools/clippy/clippy_utils/src/qualify_min_const_fn.rs Outdated

oli-obk reviewed Feb 25, 2023
View reviewed changes

Comment threadcompiler/rustc_borrowck/src/type_check/mod.rs Outdated

scottmcm added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 25, 2023

scottmcm force-pushed the mir-transmute branch from c6bce48 to 6025fc9Compare February 25, 2023 22:38

scottmcm commented Feb 25, 2023
View reviewed changes

Comment threadtests/ui/consts/const-eval/validate_uninhabited_zsts.64bit.stderr Outdated

This comment has been minimized.
Sign in to view

scottmcm force-pushed the mir-transmute branch from 6025fc9 to ff006d2Compare February 25, 2023 22:52

This comment has been minimized.
Sign in to view

scottmcm force-pushed the mir-transmute branch from ff006d2 to 0905badCompare February 25, 2023 23:32

scottmcm added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 25, 2023

RalfJung reviewed Feb 26, 2023
View reviewed changes

Comment threadcompiler/rustc_mir_transform/src/lower_intrinsics.rs Outdated

JakobDegen reviewed Feb 27, 2023
View reviewed changes

Comment threadcompiler/rustc_borrowck/src/type_check/mod.rs

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 Feb 28, 2023

matthiaskrgr mentioned this pull request Mar 1, 2023
Rollup of 7 pull requests #108600
Closed

oli-obk reviewed Mar 23, 2023
View reviewed changes

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 Mar 23, 2023

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Mar 23, 2023

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 Mar 23, 2023

bors added the merged-by-bors This PR was explicitly merged by bors. label Mar 23, 2023

bors merged commit e216300 into rust-lang:masterMar 23, 2023

rustbot added this to the 1.70.0 milestone Mar 23, 2023

scottmcm deleted the mir-transmute branch March 23, 2023 21:22

bors mentioned this pull request Mar 23, 2023
Rollup of 12 pull requests #109490
Closed

rustbot added the perf-regression Performance regression. label Mar 23, 2023

This was referenced Mar 24, 2023
Avoid some allocas in codegen of CastKind::Transmute#109549
Closed
Simplify transmutes in MIR InstCombine #109612
Merged

scottmcm mentioned this pull request Apr 23, 2023
Add intrinsics::transmute_unchecked#110706
Merged

scottmcm mentioned this pull request Jul 4, 2025
Block SIMD in transmute_immediate; delete OperandValueKind#143410
Merged

		let size_in_bytes = src.layout.size.bytes();
		if size_in_bytes == 0 {

	let size_in_bytes = src.layout.size.bytes();
	if size_in_bytes == 0{
	if src.layout.is_zst(){

		// These cases are statically rejected by `mem::transmute`, so we need custom
		// MIR to be able to get to constant evaluation.

Uh oh!

Uh oh!

Conversation

scottmcm commented Feb 25, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Feb 25, 2023

Uh oh!

rustbot commented Feb 25, 2023

Uh oh!

rustbot commented Feb 25, 2023

Uh oh!

Uh oh!

Uh oh!

oli-obk commented Feb 25, 2023

Uh oh!

Uh oh!

This comment has been minimized.

This comment has been minimized.

rustbot commented Feb 25, 2023

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

scottmcm commented Feb 27, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

oli-obk commented Feb 28, 2023

Uh oh!

bors commented Feb 28, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

oli-obk commented Mar 23, 2023

Uh oh!

bors commented Mar 23, 2023

Uh oh!

bors commented Mar 23, 2023

Uh oh!

bors commented Mar 23, 2023

Uh oh!

rust-log-analyzer commented Mar 23, 2023

Uh oh!

oli-obk commented Mar 23, 2023

Uh oh!

bors commented Mar 23, 2023

Uh oh!

bors commented Mar 23, 2023

Uh oh!

rust-timer commented Mar 23, 2023

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

Uh oh!

rylev commented Mar 28, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

14 participants

scottmcm commented Feb 25, 2023•
edited
Loading

scottmcm commented Feb 27, 2023•
edited
Loading