Stream: t-compiler/wg-mir-opt

Topic: Low hanging fruit?


Christian Poveda (May 24 2019 at 21:44, on Zulip):

Hey hi, I've been following the constant propagation efforts here. Do you think there is some kind of low hanging fruit to work on?

oli (May 24 2019 at 21:54, on Zulip):

hmm... @Wesley Wiser is munching away at const prop right now. They probably have a better picture of the situation right now

Wesley Wiser (May 25 2019 at 02:06, on Zulip):

So I'm working on implementing support for the Len operand. Here's the other stuff I'm aware of but not (yet) actively working on: support for Immediates, adding more tests in src/test/mir-opt/const-prop/, Zoxc mentioned moving const-prop earlier so that we can handle Yield termiantors.

Wesley Wiser (May 25 2019 at 02:07, on Zulip):

There's probably other stuff but that's all I can think of for now

Christian Poveda (May 25 2019 at 06:02, on Zulip):

what should happen when an Immediate inner scalar is undef?

Wesley Wiser (May 25 2019 at 07:17, on Zulip):

We should stop trying to const-prop that value if it's undef. Most of the eval_* functions return Option so you can just return None in that case.

@oli Correct me if I'm wrong ^

Christian Poveda (May 25 2019 at 17:36, on Zulip):

I'd like to take care of adding support for Immediates then

Wesley Wiser (May 25 2019 at 20:19, on Zulip):

Sorry, I just realized, when I was saying Immediate, what I actually meant was Indirect https://doc.rust-lang.org/nightly/nightly-rustc/rustc_mir/interpret/enum.Operand.html#variant.Indirect

Wesley Wiser (May 25 2019 at 20:19, on Zulip):

We already handle Immediate

Wesley Wiser (May 25 2019 at 20:20, on Zulip):

The code you need to change is in this function https://github.com/rust-lang/rust/blob/02f5786a324c40b2d8b2d0df98456e48fb45d30c/src/librustc_mir/transform/const_prop.rs#L517

Christian Poveda (May 27 2019 at 14:11, on Zulip):

Great! I'll start reading a little bit and come back with questions :)

Christian Poveda (May 30 2019 at 03:40, on Zulip):

At the end this should get the MemPlace inside the Indirect and using the Scalar inside it, update rval as in https://github.com/rust-lang/rust/blob/02f5786a324c40b2d8b2d0df98456e48fb45d30c/src/librustc_mir/transform/const_prop.rs#L528, right?

oli (May 30 2019 at 07:19, on Zulip):

you need to read the scalar from the MemPlace first

oli (May 30 2019 at 07:19, on Zulip):

the MemPlace just refers to memory

oli (May 30 2019 at 07:19, on Zulip):

so that "Scalar" is actually a pointer into memory

oli (May 30 2019 at 07:19, on Zulip):

may just be an integer in the case of ZSTs

oli (May 30 2019 at 07:20, on Zulip):

because reading a ZST doesn't actually read from memory, so you don't need actual backing memory

Christian Poveda (May 30 2019 at 17:32, on Zulip):

And how do I get the real scalar that the memplace's scalar is pointing to?

Wesley Wiser (May 31 2019 at 00:35, on Zulip):

I think you want deref_operand

Christian Poveda (May 31 2019 at 00:49, on Zulip):

great!

Wesley Wiser (May 31 2019 at 19:23, on Zulip):

@Christian Poveda Let me know if you have any questions. I have some free time tonight/tomorrow if you need any assistance :slight_smile:

Christian Poveda (May 31 2019 at 19:27, on Zulip):

Are you sure you aren't a magician? I was just creating the branch to start working on this hahahaha

Christian Poveda (May 31 2019 at 19:51, on Zulip):

Ok more or less i have this

Christian Poveda (May 31 2019 at 19:53, on Zulip):

to use deref_operand i need an OpTy, which can be obtained using eval_operand, to get the operand I used operand_with_scalar(ptr, value.layout.ty, span) where ptr is the scalar inside the MemPlace.

Wesley Wiser (May 31 2019 at 19:53, on Zulip):

That looks correct to me yeah

Christian Poveda (May 31 2019 at 19:54, on Zulip):

now the thing is, deref_operand returns an MPlaceTy, how do I produce an Rvalue from it?

Wesley Wiser (May 31 2019 at 20:01, on Zulip):

So

Wesley Wiser (May 31 2019 at 20:01, on Zulip):

You need to do something kind of like what we do here https://github.com/rust-lang/rust/blob/75f464481ed8c924086fc0b9a2d31841bbdbcabd/src/librustc_mir/transform/const_prop.rs#L504

Wesley Wiser (May 31 2019 at 20:03, on Zulip):

But instead of ty::Const::from_scalar(), you need to create a Const whose value is ConstValue::ByRef https://doc.rust-lang.org/nightly/nightly-rustc/rustc/mir/interpret/enum.ConstValue.html#variant.ByRef

Wesley Wiser (May 31 2019 at 20:04, on Zulip):

If that makes sense...

Christian Poveda (May 31 2019 at 20:11, on Zulip):

If that makes sense...

what do you mean?

Wesley Wiser (May 31 2019 at 20:17, on Zulip):

Basically you need to add a function similar to operand_from_scalar but for references (operand_from_ref maybe?)

Wesley Wiser (May 31 2019 at 20:18, on Zulip):

The main difference between these functions is going to be what the ty::Const value is

Wesley Wiser (May 31 2019 at 20:18, on Zulip):

Here we create a Const which points to a scalar value https://github.com/rust-lang/rust/blob/75f464481ed8c924086fc0b9a2d31841bbdbcabd/src/librustc_mir/transform/const_prop.rs#L510

Wesley Wiser (May 31 2019 at 20:18, on Zulip):

But in the new function, we need to create a Const which points to that MPlace

Wesley Wiser (May 31 2019 at 20:19, on Zulip):

There should be a way to get a Pointer and Allocation from the MPlace you have

Christian Poveda (May 31 2019 at 20:22, on Zulip):

Ok let me see and I'll come back to you

Christian Poveda (May 31 2019 at 20:28, on Zulip):

But in the new function, we need to create a Const which points to that MPlace

When you say that MPlace you mean the MPlaceTy, right?

Wesley Wiser (May 31 2019 at 20:29, on Zulip):

Yeah

Wesley Wiser (May 31 2019 at 20:30, on Zulip):

MPlaceTy is basically just a tuple of (MPlace, TyLayout)

Christian Poveda (May 31 2019 at 20:30, on Zulip):

ok :P

Christian Poveda (May 31 2019 at 20:34, on Zulip):

Ok i can get the Pointer from MPlaceTy::vtable but this return a result, how are errors handled here? should operand_from_ref return a result?

Wesley Wiser (May 31 2019 at 20:37, on Zulip):

It can probably just return Option<Operand<'tcx>>

Wesley Wiser (May 31 2019 at 20:37, on Zulip):

Then the calling code can use self.operand_from_ref(...)?

Christian Poveda (May 31 2019 at 20:42, on Zulip):

mmm... but replace_with_const returns ()

Wesley Wiser (May 31 2019 at 20:45, on Zulip):

oh, good point

Christian Poveda (May 31 2019 at 20:45, on Zulip):

to use deref_operand i need an OpTy, which can be obtained using eval_operand, to get the operand I used operand_with_scalar(ptr, value.layout.ty, span) where ptr is the scalar inside the MemPlace.

also to call eval_operand in need the source_info

Wesley Wiser (May 31 2019 at 20:45, on Zulip):

You can do if let Some(...) = self.operand_from_ref(...) then

Wesley Wiser (May 31 2019 at 20:45, on Zulip):

If we get None that's not an error, it just means we can't const propagate

Christian Poveda (May 31 2019 at 20:48, on Zulip):

oh damn and vtable is private, i cannot use it

Wesley Wiser (May 31 2019 at 20:49, on Zulip):

Do you need vtable?

Christian Poveda (May 31 2019 at 20:50, on Zulip):

well, I used to get the Pointer from the MPlaceTy

Wesley Wiser (May 31 2019 at 20:52, on Zulip):

Hmm... I'm not sure I'm following

Wesley Wiser (May 31 2019 at 20:52, on Zulip):

Can you explain a bit more?

Christian Poveda (May 31 2019 at 20:52, on Zulip):

yeah this is messy

Christian Poveda (May 31 2019 at 20:52, on Zulip):

let me see

Wesley Wiser (May 31 2019 at 20:53, on Zulip):

Oh do you mean MPlaceTy::vtable()?

Christian Poveda (May 31 2019 at 20:53, on Zulip):

yep

Wesley Wiser (May 31 2019 at 20:53, on Zulip):

I don't think that's what you want

Christian Poveda (May 31 2019 at 20:54, on Zulip):

should i just call to_ptr?

Wesley Wiser (May 31 2019 at 20:54, on Zulip):

That (vtable()) will give you a pointer to the object's vtable (if that's a valid operation for the kind of reference you have)

Wesley Wiser (May 31 2019 at 20:54, on Zulip):

Yeah, I think to_ptr is what you want

oli (Nov 09 2019 at 00:41, on Zulip):

Not sure if it counts as a low hanging fruit, but https://github.com/rust-lang/rust/issues/66234 at least has parts that shouldn't be enormously hard to implement

Christian Poveda (Nov 09 2019 at 06:41, on Zulip):

count me in :)

centril (Nov 11 2019 at 08:14, on Zulip):

Filed https://github.com/rust-lang/rust/pull/66282 :slight_smile:

Last update: Nov 17 2019 at 06:55UTC