Lower matches against constant slices into better MIR #112370

john-h-k · 2023-06-07T01:10:43Z

Fixes #110870 (eventually)

The approach I've been trying so far is lowering PatKind::Array (when it is a fixed array with only constants in it) to a constant pattern in mir_build. Currently there are two issues here:

I am hitting an assertion around unexpected types for constants
I don't know how to allocate the Pat into the tcx to get the lifetimes right (hence the Box::leak hack)

I will continue toying around with this but wanted to open the PR to see if this is the wrong way of doing it. Should I be doing it in a different MIR stage/a MIR opt pass?

rustbot · 2023-06-07T01:10:51Z

r? @eholk

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

john-h-k · 2023-06-07T04:50:34Z

~~Okay, it looks like currently Constant really means u128 or smaller constants, so it might not be the right type to approach this with~~

john-h-k · 2023-06-07T08:43:30Z

Okay, I have somehow managed to get the basic test case for this working with the expected MIR generation. The basic idea is (this still may be the entirely wrong way to do this):

During THIR -> MIR lowering, PatKind::Arrays which meet these criteria are recognised and handled as this special case
- No slice element (not sure what it is called, but as in no [1, .., 2]
- No suffix (as when there is no slice element I am assuming there is no suffix)
- All elements within prefix are PatKind::Constant
The handling for this case is
- Construct a ValTree from the elements of the array,
Things I still need help with
- Is this the right way to do it?!
- How do I allocate the new pattern properly (rather than the Box::leak hack I currently have) with the `'tcx' lifetime in compiler/rustc_mir_build/src/build/matches/simplify.rs:316
- Is the type logic for the pattern ([<type of the constant in the array>; <count in array>] correct? Or should it be based on the match type?

WaffleLapkin · 2023-06-08T17:02:27Z

r? mir-opt

rustbot · 2023-06-16T09:53:41Z

Some changes occurred in compiler/rustc_codegen_gcc

cc @antoyo

antoyo · 2023-06-16T16:43:08Z

compiler/rustc_codegen_gcc/src/lib.rs

@@ -35,7 +35,7 @@ extern crate rustc_middle;
 extern crate rustc_session;
 extern crate rustc_span;
 extern crate rustc_target;
-extern crate tempfile;
+


tempfile is used at line 89 so it should not be removed.

Not sure why there's a warning emitted about this, but removing this line will result in a compilation error.

bors · 2023-06-29T10:50:31Z

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

cjgillot

This looks like the right place to put this change. Could you add a MIR building test, in the tests/mir-opt/build directory, so we can see the results of this change?

cjgillot · 2023-06-25T14:05:01Z

compiler/rustc_middle/src/ty/consts.rs

@@ -18,7 +18,7 @@ pub use valtree::*;
 /// Use this rather than `ConstData`, whenever possible.
 #[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, HashStable)]
 #[rustc_pass_by_value]
-pub struct Const<'tcx>(pub(super) Interned<'tcx, ConstData<'tcx>>);
+pub struct Const<'tcx>(pub Interned<'tcx, ConstData<'tcx>>);


this has still not been addressed

cjgillot · 2023-06-25T14:09:03Z

compiler/rustc_mir_build/src/build/matches/simplify.rs

+                ));
+
+                Ok(())
+            }


Should this special case be moved to prefix_slice_suffix? Once for the prefix and once for the suffix?

cjgillot · 2023-06-25T14:14:25Z

compiler/rustc_mir_build/src/build/matches/simplify.rs

+
+                candidate.match_pairs.push(MatchPair::new(
+                    place,
+                    Box::leak(Box::new(new_pat)), // FIXME: 'tcx lifetime for pat


You should add rustc_middle::thir::Pat to the list in rustc_middle::arena, and use tcx.arena.alloc.

compiler/rustc_mir_build/src/build/matches/simplify.rs

cjgillot · 2023-07-01T12:51:13Z

cc @oli-obk as you manipulated that code recently.

compiler/rustc_mir_build/src/build/matches/simplify.rs

Dylan-DPC · 2023-08-15T17:55:19Z

@john-h-k any updates on this?

john-h-k · 2023-08-31T13:30:34Z

@john-h-k any updates on this?

Sorry have been super busy the last few weeks - I am currently working on this yes

rustbot · 2023-08-31T23:16:16Z

Some changes occurred in src/tools/cargo

cc @ehuss

oli-obk · 2023-09-04T14:22:21Z

compiler/rustc_middle/src/ty/consts.rs

+    #[inline]
+    pub fn new(tcx: TyCtxt<'tcx>, kind: ty::ConstKind<'tcx>, ty: Ty<'tcx>) -> Const<'tcx> {
+        tcx.mk_ct_from_kind(kind, ty)
+    }
+
+    #[inline]
+    pub fn new_param(tcx: TyCtxt<'tcx>, param: ty::ParamConst, ty: Ty<'tcx>) -> Const<'tcx> {
+        Const::new(tcx, ty::ConstKind::Param(param), ty)
+    }
+
+    #[inline]
+    pub fn new_var(tcx: TyCtxt<'tcx>, infer: ty::ConstVid<'tcx>, ty: Ty<'tcx>) -> Const<'tcx> {
+        Const::new(tcx, ty::ConstKind::Infer(ty::InferConst::Var(infer)), ty)
+    }
+
+    #[inline]
+    pub fn new_fresh(tcx: TyCtxt<'tcx>, fresh: u32, ty: Ty<'tcx>) -> Const<'tcx> {
+        Const::new(tcx, ty::ConstKind::Infer(ty::InferConst::Fresh(fresh)), ty)
+    }
+
+    #[inline]
+    pub fn new_infer(tcx: TyCtxt<'tcx>, infer: ty::InferConst<'tcx>, ty: Ty<'tcx>) -> Const<'tcx> {
+        Const::new(tcx, ty::ConstKind::Infer(infer), ty)
+    }
+
+    #[inline]
+    pub fn new_bound(
+        tcx: TyCtxt<'tcx>,
+        debruijn: ty::DebruijnIndex,
+        var: ty::BoundVar,
+        ty: Ty<'tcx>,
+    ) -> Const<'tcx> {
+        Const::new(tcx, ty::ConstKind::Bound(debruijn, var), ty)
+    }
+
+    #[inline]
+    pub fn new_placeholder(
+        tcx: TyCtxt<'tcx>,
+        placeholder: ty::PlaceholderConst<'tcx>,
+        ty: Ty<'tcx>,
+    ) -> Const<'tcx> {
+        Const::new(tcx, ty::ConstKind::Placeholder(placeholder), ty)
+    }
+
+    #[inline]
+    pub fn new_unevaluated(
+        tcx: TyCtxt<'tcx>,
+        uv: ty::UnevaluatedConst<'tcx>,
+        ty: Ty<'tcx>,
+    ) -> Const<'tcx> {
+        Const::new(tcx, ty::ConstKind::Unevaluated(uv), ty)
+    }
+
+    #[inline]
+    pub fn new_value(tcx: TyCtxt<'tcx>, val: ty::ValTree<'tcx>, ty: Ty<'tcx>) -> Const<'tcx> {
+        Const::new(tcx, ty::ConstKind::Value(val), ty)
+    }
+
+    #[inline]
+    pub fn new_expr(tcx: TyCtxt<'tcx>, expr: ty::Expr<'tcx>, ty: Ty<'tcx>) -> Const<'tcx> {
+        Const::new(tcx, ty::ConstKind::Expr(expr), ty)
+    }
+
+    #[inline]
+    pub fn new_error(tcx: TyCtxt<'tcx>, e: ty::ErrorGuaranteed, ty: Ty<'tcx>) -> Const<'tcx> {
+        Const::new(tcx, ty::ConstKind::Error(e), ty)
+    }
+
+    /// Like [Ty::new_error] but for constants.
+    #[track_caller]
+    pub fn new_misc_error(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>) -> Const<'tcx> {
+        Const::new_error_with_message(
+            tcx,
+            ty,
+            DUMMY_SP,
+            "ty::ConstKind::Error constructed but no error reported",
+        )
+    }


This should probably be its own PR, so we aren't mixing a restructuring with a behaviour change

john-h-k · 2023-09-04T20:19:26Z

@rustbot review

oli-obk · 2023-09-05T07:36:50Z

Please add a mir-opt test that shows the bad MIR in a commit before your main commit (which should also get a meaningful commit message). Then your commit will change the MIR of the new test, and thus demonstrate how your PR changes things.

compiler/rustc_middle/src/ty/consts/valtree.rs

oli-obk · 2023-09-05T07:40:35Z

@rustbot author

bors · 2023-09-21T14:26:10Z

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

Co-authored-by: Oli Scherer <github35764891676564198441@oli-obk.de>

JohnCSimon · 2023-11-12T18:41:49Z

@john-h-k
ping from triage - can you post your status on this PR? There hasn't been an update in a few months. Thanks!

FYI: when a PR is ready for review, send a message containing
@rustbot ready to switch to S-waiting-on-review so the PR is in the reviewer's backlog.

JohnCSimon · 2024-02-11T21:39:04Z

@john-h-k

Ping from triage: I'm closing this due to inactivity, Please reopen when you are ready to continue with this.
Note: if you are going to continue please open the PR BEFORE you push to it, else you won't be able to reopen - this is a quirk of github.
Thanks for your contribution.

@rustbot label: +S-inactive

rustbot assigned eholk Jun 7, 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 Jun 7, 2023

This comment has been minimized.

Sign in to view

rustbot assigned wesleywiser and unassigned eholk Jun 8, 2023

This comment has been minimized.

Sign in to view

antoyo reviewed Jun 16, 2023

View reviewed changes

cjgillot reviewed Jul 1, 2023

View reviewed changes

cjgillot self-assigned this Jul 1, 2023

cjgillot 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 Jul 4, 2023

oli-obk requested changes Jul 5, 2023

View reviewed changes

compiler/rustc_mir_build/src/build/matches/simplify.rs Outdated Show resolved Hide resolved

john-h-k force-pushed the compiler/bad-slice-match branch 3 times, most recently from 63581fb to 0c77221 Compare August 31, 2023 23:05

rustbot added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Sep 1, 2023

john-h-k force-pushed the compiler/bad-slice-match branch 7 times, most recently from 46be8c1 to a8a195d Compare September 4, 2023 13:34

oli-obk reviewed Sep 4, 2023

View reviewed changes

john-h-k force-pushed the compiler/bad-slice-match branch 2 times, most recently from 69dbd6b to 1cdaa59 Compare September 4, 2023 19:38

This comment has been minimized.

Sign in to view

redo git history

89ff2b2

john-h-k force-pushed the compiler/bad-slice-match branch from 50a2655 to 89ff2b2 Compare September 4, 2023 20:11

This comment has been minimized.

Sign in to view

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

oli-obk reviewed Sep 5, 2023

View reviewed changes

compiler/rustc_middle/src/ty/consts/valtree.rs Outdated Show resolved Hide resolved

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 5, 2023

Update compiler/rustc_middle/src/ty/consts/valtree.rs

465d3f4

Co-authored-by: Oli Scherer <github35764891676564198441@oli-obk.de>

JohnCSimon closed this Feb 11, 2024

rustbot added the S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. label Feb 11, 2024

jieyouxu mentioned this pull request Feb 24, 2024

[EXPERIMENTAL] Lower constant pattern subslices into valtrees #121540

Closed

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Lower matches against constant slices into better MIR #112370

Lower matches against constant slices into better MIR #112370

john-h-k commented Jun 7, 2023

rustbot commented Jun 7, 2023

This comment has been minimized.

john-h-k commented Jun 7, 2023 •

edited

Loading

john-h-k commented Jun 7, 2023

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

WaffleLapkin commented Jun 8, 2023

rustbot commented Jun 16, 2023

This comment has been minimized.

antoyo Jun 16, 2023

bors commented Jun 29, 2023

cjgillot left a comment

cjgillot Jun 25, 2023

oli-obk Sep 5, 2023

cjgillot Jun 25, 2023

cjgillot Jun 25, 2023

cjgillot commented Jul 1, 2023

Dylan-DPC commented Aug 15, 2023

john-h-k commented Aug 31, 2023

rustbot commented Aug 31, 2023

oli-obk Sep 4, 2023

This comment has been minimized.

This comment has been minimized.

john-h-k commented Sep 4, 2023

oli-obk commented Sep 5, 2023

oli-obk commented Sep 5, 2023

bors commented Sep 21, 2023

JohnCSimon commented Nov 12, 2023

JohnCSimon commented Feb 11, 2024

+                              ));
+                              Ok(())
+                          }

Lower matches against constant slices into better MIR #112370

Lower matches against constant slices into better MIR #112370

Conversation

john-h-k commented Jun 7, 2023

rustbot commented Jun 7, 2023

This comment has been minimized.

john-h-k commented Jun 7, 2023 • edited Loading

john-h-k commented Jun 7, 2023

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

WaffleLapkin commented Jun 8, 2023

rustbot commented Jun 16, 2023

This comment has been minimized.

antoyo Jun 16, 2023

Choose a reason for hiding this comment

bors commented Jun 29, 2023

cjgillot left a comment

Choose a reason for hiding this comment

cjgillot Jun 25, 2023

Choose a reason for hiding this comment

oli-obk Sep 5, 2023

Choose a reason for hiding this comment

cjgillot Jun 25, 2023

Choose a reason for hiding this comment

cjgillot Jun 25, 2023

Choose a reason for hiding this comment

cjgillot commented Jul 1, 2023

Dylan-DPC commented Aug 15, 2023

john-h-k commented Aug 31, 2023

rustbot commented Aug 31, 2023

oli-obk Sep 4, 2023

Choose a reason for hiding this comment

This comment has been minimized.

This comment has been minimized.

john-h-k commented Sep 4, 2023

oli-obk commented Sep 5, 2023

oli-obk commented Sep 5, 2023

bors commented Sep 21, 2023

JohnCSimon commented Nov 12, 2023

JohnCSimon commented Feb 11, 2024

john-h-k commented Jun 7, 2023 •

edited

Loading