Generate aggregate constants in DataflowConstProp. #115796

cjgillot · 2023-09-12T17:39:47Z

No description provided.

rustbot · 2023-09-12T17:39:53Z

r? @compiler-errors

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

rustbot · 2023-09-12T17:39:56Z

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

cjgillot · 2023-09-13T09:53:04Z

compiler/rustc_mir_transform/src/dataflow_const_prop.rs

+        }
+    }
+
+    fn try_make_valtree(


I need to duplicate this method into a check_ version and a make_ version. The try_make_ version leaks memory if it fails to complete the ValTree, which is more than likely in general.

Hmm... Maybe we should have some sort of OwnedValtree that can infallibly be converted to a Valtree. This way you can create the OwnedValtree and dropping it frees all memory?

This function looks rather similar to const_to_valtree_inner.

Maybe we should make that function generic over how to read fields so the logic is not duplicated?

cjgillot · 2023-09-13T09:55:06Z

compiler/rustc_mir_transform/src/dataflow_const_prop.rs

+                    let discr_bits = discr.assert_bits(discr.size());
+                    let (variant, _) =
+                        def.discriminants(self.tcx).find(|(_, var)| discr_bits == var.val)?;
+                    let variant_place = map.apply(place, TrackElem::Variant(variant))?;
+                    let variant_int = ty::ValTree::Leaf(variant.as_u32().into());


@oli-obk Why do valtrees encode the VariantIdx and not the discriminant value?
Using the discriminant would make them easier to translate from/to runtime representation. Would it hinder some other usage?

There's no reason. I think it was just the value that was immediately available

bors · 2023-09-13T19:20:04Z

☔ The latest upstream changes (presumably #115797) made this pull request unmergeable. Please resolve the merge conflicts.

compiler-errors · 2023-09-16T19:53:54Z

r? @oli-obk

oli-obk · 2023-09-18T09:57:43Z

@rustbot author (at least i think you still wanted to do something here?)

bors · 2023-09-21T15:00:08Z

☔ The latest upstream changes (presumably #116027) made this pull request unmergeable. Please resolve the merge conflicts.

cjgillot · 2023-10-22T13:39:20Z

The latest version drops the idea of using ValTrees. It uses plain ConstValue instead, which is much simpler.

oli-obk · 2023-10-23T08:27:07Z

compiler/rustc_mir_transform/src/dataflow_const_prop.rs

+
+    match ty.kind() {
+        // ZSTs. Nothing to do.
+        ty::FnDef(..) => {}


Suggested change

ty::FnDef(..) => {}

ty::FnDef(..) => bug!(),

oli-obk · 2023-10-23T08:28:30Z

compiler/rustc_mir_transform/src/dataflow_const_prop.rs

+        ty::FnDef(..) => {}
+
+        // Those are scalars, must be handled above.
+        ty::Bool | ty::Int(_) | ty::Uint(_) | ty::Float(_) | ty::Char => throw_machine_stop_str!("primitive type with provenance"),


This is possible in case of a pointer-to-int transmute. We don't strip provenance when transmuting, so the state is a Scalar::Ptr, which is rejected by propagatable_scalar.

oli-obk · 2023-10-23T10:52:33Z

@bors r+

bors · 2023-10-23T10:52:35Z

📌 Commit 8c1b039 has been approved by oli-obk

It is now in the queue for this repository.

oli-obk · 2023-10-23T10:52:56Z

@bors r- oh wait we should perf it

oli-obk · 2023-10-23T14:46:22Z

@bors r+ rollup

bors · 2023-10-23T14:46:24Z

📌 Commit 8c1b039 has been approved by oli-obk

It is now in the queue for this repository.

Generate aggregate constants in DataflowConstProp.

…iaskrgr Rollup of 6 pull requests Successful merges: - rust-lang#115796 (Generate aggregate constants in DataflowConstProp.) - rust-lang#116859 (Make `ty::print::Printer` take `&mut self` instead of `self`) - rust-lang#117046 (return unfixed len if pat has reported error) - rust-lang#117070 (rustdoc: wrap Type with Box instead of Generics) - rust-lang#117074 (Remove smir from triage and add me to stablemir) - rust-lang#117086 (Update .mailmap to promote my livename) r? `@ghost` `@rustbot` modify labels: rollup

matthiaskrgr · 2023-10-23T20:25:35Z

@bors r- failed in a rollup
#117098 (comment)

oli-obk · 2023-10-24T15:31:07Z

@bors r+ rollup=never some conflict prone tests

bors · 2023-10-24T15:31:10Z

📌 Commit 9c85dfa has been approved by oli-obk

It is now in the queue for this repository.

bors · 2023-10-24T21:47:56Z

⌛ Testing commit 9c85dfa with merge df871fb...

bors · 2023-10-24T23:47:10Z

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

rust-timer · 2023-10-25T00:58:01Z

Finished benchmarking commit (df871fb): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

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

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)	4.1%	[4.1%, 4.1%]	1
Regressions ❌ (secondary)	2.2%	[2.2%, 2.3%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	4.1%	[4.1%, 4.1%]	1

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)	-2.3%	[-2.3%, -2.3%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-2.3%	[-2.3%, -2.3%]	1

Binary size

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

Bootstrap: missing data
Artifact size: 304.37 MiB -> 304.40 MiB (0.01%)

rustbot assigned compiler-errors Sep 12, 2023

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 Sep 12, 2023

cjgillot added the A-mir-opt Area: MIR optimizations label Sep 12, 2023

cjgillot commented Sep 13, 2023

View reviewed changes

cjgillot force-pushed the const-prop-rvalue branch from ac276e4 to 7f65502 Compare September 13, 2023 21:04

rustbot assigned oli-obk and unassigned compiler-errors Sep 16, 2023

rustbot 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 Sep 18, 2023

cjgillot force-pushed the const-prop-rvalue branch from 7f65502 to 5d9e2c8 Compare September 18, 2023 18:27

cjgillot changed the title ~~Generate ValTrees in DataflowConstProp.~~ Generate aggregate constants in DataflowConstProp. Sep 18, 2023

cjgillot force-pushed the const-prop-rvalue branch from 5d9e2c8 to 5f75825 Compare September 20, 2023 18:39

cjgillot added 2 commits October 21, 2023 16:20

Generate ValTrees in DataflowConstProp.

31d1010

Use a ConstValue instead.

8c1b039

cjgillot force-pushed the const-prop-rvalue branch from 5f75825 to 8c1b039 Compare October 22, 2023 12:14

oli-obk reviewed Oct 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 23, 2023

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Oct 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 23, 2023

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Oct 23, 2023

Rollup merge of rust-lang#115796 - cjgillot:const-prop-rvalue, r=oli-obk

7d0a7de

Generate aggregate constants in DataflowConstProp.

matthiaskrgr mentioned this pull request Oct 23, 2023

Rollup of 7 pull requests #117096

Closed

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Oct 23, 2023

Rollup merge of rust-lang#115796 - cjgillot:const-prop-rvalue, r=oli-obk

d07f2e0

Generate aggregate constants in DataflowConstProp.

matthiaskrgr mentioned this pull request Oct 23, 2023

Rollup of 6 pull requests #117098

Closed

bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Oct 23, 2023

Tweak test to avoid platform dependency.

9c85dfa

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 24, 2023

bors added the merged-by-bors This PR was explicitly merged by bors. label Oct 24, 2023

bors merged commit df871fb into rust-lang:master Oct 24, 2023
11 checks passed

rustbot added this to the 1.75.0 milestone Oct 24, 2023

This was referenced Oct 25, 2023

Perform opportunistic simplifications during value numbering #111344

Closed

See through aggregates in GVN #116270

Merged

cjgillot deleted the const-prop-rvalue branch October 25, 2023 06:38

github-actions bot mentioned this pull request Oct 31, 2023

Automated pull from upstream master ferrocene/ferrocene#74

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Generate aggregate constants in DataflowConstProp. #115796

Generate aggregate constants in DataflowConstProp. #115796

cjgillot commented Sep 12, 2023

rustbot commented Sep 12, 2023

rustbot commented Sep 12, 2023

cjgillot Sep 13, 2023

oli-obk Sep 13, 2023

oli-obk Sep 13, 2023

cjgillot Sep 13, 2023

oli-obk Sep 13, 2023

bors commented Sep 13, 2023

compiler-errors commented Sep 16, 2023

oli-obk commented Sep 18, 2023

bors commented Sep 21, 2023

cjgillot commented Oct 22, 2023

oli-obk Oct 23, 2023

oli-obk Oct 23, 2023

cjgillot Oct 23, 2023

oli-obk commented Oct 23, 2023

bors commented Oct 23, 2023

oli-obk commented Oct 23, 2023

oli-obk commented Oct 23, 2023

bors commented Oct 23, 2023

matthiaskrgr commented Oct 23, 2023

oli-obk commented Oct 24, 2023

bors commented Oct 24, 2023

bors commented Oct 24, 2023

bors commented Oct 24, 2023

rust-timer commented Oct 25, 2023

	ty::Bool \| ty::Int(_) \| ty::Uint(_) \| ty::Float(_) \| ty::Char => throw_machine_stop_str!("primitive type with provenance"),
	ty::Bool \| ty::Int(_) \| ty::Uint(_) \| ty::Float(_) \| ty::Char => bug!("primitive type with provenance"),

Generate aggregate constants in DataflowConstProp. #115796

Generate aggregate constants in DataflowConstProp. #115796

Conversation

cjgillot commented Sep 12, 2023

rustbot commented Sep 12, 2023

rustbot commented Sep 12, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bors commented Sep 13, 2023

compiler-errors commented Sep 16, 2023

oli-obk commented Sep 18, 2023

bors commented Sep 21, 2023

cjgillot commented Oct 22, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oli-obk commented Oct 23, 2023

bors commented Oct 23, 2023

oli-obk commented Oct 23, 2023

oli-obk commented Oct 23, 2023

bors commented Oct 23, 2023

matthiaskrgr commented Oct 23, 2023

oli-obk commented Oct 24, 2023

bors commented Oct 24, 2023

bors commented Oct 24, 2023

bors commented Oct 24, 2023

rust-timer commented Oct 25, 2023

Overall result: no relevant changes - no action needed