Stream: t-compiler/help

Topic: Dataflow question


Amanieu (Feb 25 2020 at 23:08, on Zulip):

In src/librustc_mir/dataflow/impls/borrows.rs why does Terminator::Call not need to call kill_borrows_on_place() for its destination place, while Statement::Assign does?

Amanieu (Feb 25 2020 at 23:09, on Zulip):

I'm trying to understand this code to figure out if I need to add handling for Terminator::InlineAsm, which I am adding.

Amanieu (Feb 25 2020 at 23:09, on Zulip):

cc @ecstatic-morse (@eddyb suggested you might be familiar with this code)

ecstatic-morse (Feb 25 2020 at 23:27, on Zulip):

@Amanieu No idea. It seems like it should kill any borrows of the return place in call_return_effect. This wasn't implemented by me, as the semantics are unchanged from at least 1.35, which was before I worked on rust. You probably need to ask @pnkfelix.

ecstatic-morse (Feb 25 2020 at 23:40, on Zulip):

Is the goal to eventually support unwinding out of InlineAsm?

Amanieu (Feb 25 2020 at 23:44, on Zulip):

Maybe in the future. The more immediate reason is to support diverging InlineAsm

ecstatic-morse (Feb 25 2020 at 23:46, on Zulip):

But you want a "success" edge and an "unwind" edge like Call, correct?

Amanieu (Feb 25 2020 at 23:48, on Zulip):

Not really, just a success edge for now.

Amanieu (Feb 25 2020 at 23:49, on Zulip):

Here's the MIR part of the PR if you want to have a look: https://github.com/rust-lang/rust/pull/69171/commits/8d3b9ba81e2cebb0d2bc528ea523243723813ef9

ecstatic-morse (Feb 25 2020 at 23:51, on Zulip):

If there's only ever one edge out of InlineAsm, does it need to be converted to a terminator as part of your PR? Or can it remain a statement for now?

ecstatic-morse (Feb 25 2020 at 23:52, on Zulip):

I mean, I think it should ultimately be a terminator, but does it save work to make that change now as opposed to later?

Amanieu (Feb 25 2020 at 23:55, on Zulip):

Well I'm not using any of the existing InlineAsm code, it's more of a rewrite from scratch.

Amanieu (Feb 25 2020 at 23:55, on Zulip):

The old InlineAsm was renamed to LlvmInlineAsm (see first commit)

Amanieu (Feb 25 2020 at 23:55, on Zulip):

Since this is a rewrite, I figured it may as well be done properly.

ecstatic-morse (Feb 26 2020 at 00:00, on Zulip):

Ah, that's fine. I suspect that there are places where InlineAsm is not handled correctly in the existing code. I can't help you with Borrows unfortunately, but feel free to ask about other dataflow places. As long as InlineAsm only has one outgoing edge it should be pretty much equivalent to the statement version as far as dataflow is concerned.

ecstatic-morse (Feb 26 2020 at 00:05, on Zulip):

If InlineAsm were to have multiple outgoing edges and the edge that was taken determined which outputs were assigned to, you would need to make changes to the dataflow API.

Amanieu (Feb 26 2020 at 09:57, on Zulip):

@pnkfelix Could you have a quick look at src/librustc_mir/dataflow/impls/borrows.rs? I think call_return_effect needs to call self.kill_borrows_on_place, like StatementKind::Assign does, but I'm not very familiar with what the code does.

pnkfelix (Feb 26 2020 at 15:48, on Zulip):

yeah I'll try to take a look today, thanks for your investigation so far.

Last update: Apr 03 2020 at 16:15UTC