{"action":"created","issue":{"url":"https://api.github.com/repos/rust-lang/rust/issues/32105","repository_url":"https://api.github.com/repos/rust-lang/rust","labels_url":"https://api.github.com/repos/rust-lang/rust/issues/32105/labels{/name}","comments_url":"https://api.github.com/repos/rust-lang/rust/issues/32105/comments","events_url":"https://api.github.com/repos/rust-lang/rust/issues/32105/events","html_url":"https://github.com/rust-lang/rust/issues/32105","id":139094079,"number":32105,"title":"[MIR] Reconsider invoked function return value strategy","user":{"login":"nagisa","id":679122,"avatar_url":"https://avatars.githubusercontent.com/u/679122?v=3","gravatar_id":"","url":"https://api.github.com/users/nagisa","html_url":"https://github.com/nagisa","followers_url":"https://api.github.com/users/nagisa/followers","following_url":"https://api.github.com/users/nagisa/following{/other_user}","gists_url":"https://api.github.com/users/nagisa/gists{/gist_id}","starred_url":"https://api.github.com/users/nagisa/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/nagisa/subscriptions","organizations_url":"https://api.github.com/users/nagisa/orgs","repos_url":"https://api.github.com/users/nagisa/repos","events_url":"https://api.github.com/users/nagisa/events{/privacy}","received_events_url":"https://api.github.com/users/nagisa/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/rust-lang/rust/labels/A-mir","name":"A-mir","color":"d7e102"}],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":3,"created_at":"2016-03-07T21:07:51Z","updated_at":"2016-03-08T01:02:13Z","closed_at":null,"body":"Currently in MIR function calls look like this:\r\n\r\n```\r\ndestination = function(...) -> [...]\r\n```\r\n\r\nwhere destination is a lvalue of some sort. However this assignment is a little bit troublesome where under-the-covers we need to copy the return value and zero-fill the arguments after the call finishes.\r\n\r\nLLVM has two kind of cals `invoke` and `call`. `call` is a regular instruction, whereas `invoke` is a basic block terminator. There’s no problem with the `call` instruction, because we can always produce a bunch of post-call code after `call` and then simply `br` into the destination block. `invoke`, however, does not give us that kind of freedom and we must branch into something right after the call. (remember, we still must to copy the return value and zero-fill the arguments!).\r\n\r\nPreviously we used to generate an intermediate block and translate the copy into that, but [it is problematic] and gets even more-so with zeroing considered. Lately we’ve moved to translating drops/copies straight into the target block (the `at_start` approach) – it turns out this is wrong in its current form, because the target block can easily have [more than one predecessor](http://is.gd/jL1N4T)!\r\n\r\n[it is problematic]: https://github.com/nagisa/rust/blob/432460a6fc92e8baecbc4fa175345e78232fe2ed/src/librustc_trans/trans/mir/block.rs#L178\r\n\r\nThe options considered and not considered are:\r\n\r\n* Pre-trans pass to add empty blocks for all invokes like that and just use `at_start` approach;\r\n    1. this is pretty clean in a sense that it is fully contained within trans and we get to see whole mirmap so we can do the transformation (something that’s not possible generating blocks during trans);\r\n    2. lets us keep the `at_start` approach which is the cleanest one I’ve thought up so far; but\r\n    3. generating blocks *correctly* would be not-really-trivial.\r\n* @nikomatsakis had proposed having function return value as an rvalue which must appear inside the first statement in target block. I.e. something like\r\n\r\n    ```\r\n    bb1: function(...) -> [bb2]\r\n    bb2: destination = funcret\r\n    ```\r\n\r\n    This seems pretty clean, and we could also zero-out arguments as a part of `funcret`, but there’s a few issues with this approach that come to mind:\r\n    1. funcret cannot be removed as part of optimisations (special case in optimisation passes);\r\n    2. we’d have to somehow carry the state around trans?\r\n\r\nThat’s all my thoughts so far. This is pretty important to get fixed, because it causes llvm assertions for some code."},"comment":{"url":"https://api.github.com/repos/rust-lang/rust/issues/comments/193534792","html_url":"https://github.com/rust-lang/rust/issues/32105#issuecomment-193534792","issue_url":"https://api.github.com/repos/rust-lang/rust/issues/32105","id":193534792,"user":{"login":"Aatch","id":342416,"avatar_url":"https://avatars.githubusercontent.com/u/342416?v=3","gravatar_id":"","url":"https://api.github.com/users/Aatch","html_url":"https://github.com/Aatch","followers_url":"https://api.github.com/users/Aatch/followers","following_url":"https://api.github.com/users/Aatch/following{/other_user}","gists_url":"https://api.github.com/users/Aatch/gists{/gist_id}","starred_url":"https://api.github.com/users/Aatch/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Aatch/subscriptions","organizations_url":"https://api.github.com/users/Aatch/orgs","repos_url":"https://api.github.com/users/Aatch/repos","events_url":"https://api.github.com/users/Aatch/events{/privacy}","received_events_url":"https://api.github.com/users/Aatch/received_events","type":"User","site_admin":false},"created_at":"2016-03-08T01:02:13Z","updated_at":"2016-03-08T01:02:13Z","body":"Your description of the issue with the current `at_start` seems very similar to critical edges. Critical edges are edges in a CFG that are neither the only edge leaving a block, nor the only edge entering a block.\r\n\r\n```\r\n    bb0\r\n    / \\\r\n   /   \\\r\n  /     \\\r\nbb1    bb2\r\n |     / \\\r\n |    c   \\\r\n |   /     \\\r\n bb3      bb4\r\n```\r\n\r\nSo edge `c` here is a critical edge as it's not the only edge leaving `bb2` and it's not the edge ending `bb3`. The thing with critical edges is that you have to break them (by inserting new blocks) to add computation along them without affecting the other edges, which seems pretty similar to the issue here.\r\n\r\nIdentifying and breaking critical edges is a known problem, so it shouldn't be too hard to find algorithms for doing both. I don't know enough about what is going on here to know the best overall strategy though."}}