Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MIR] Requiring TempRef to only be assigned to once may be too restrictive #31002

Closed
nagisa opened this issue Jan 18, 2016 · 6 comments
Closed
Labels
A-MIR Area: Mid-level IR (MIR) - https://blog.rust-lang.org/2016/04/19/MIR.html

Comments

@nagisa
Copy link
Member

nagisa commented Jan 18, 2016

In MIR we want tmp* values to be SSA, however ensuring that constraint in trans (i.e. in TempRef) is wrong, I think.

Namely, for code like this:

#![feature(rustc_attrs)]
struct A(f32);
#[rustc_mir(graphviz="mir.gv")]
fn mir(f: f32) -> bool { !(f.is_nan() || f.is_infinite()) }
fn main() { println!("{:?}", mir(0.0)); }

we will generate following MIR (graphviz version, because easier to visualise):

MIR

which will result in

test.rs:17:7: 17:36 error: internal compiler error: operand const false already assigned
test.rs:17     !(f.is_nan() || f.is_infinite())
                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Note how we assign tmp0 twice, but the two branches are fully disjoint, and thus, for the purposes of data-flow tmp0 is still SSA, but for the purposes of translator tmp0 is assigned multiple times.

If we want TempRefs to stay SSA across translation of the whole function, we will need something like a phi in the MIR.

cc @nikomatsakis

@nagisa
Copy link
Member Author

nagisa commented Jan 18, 2016

Another option would be to convert these temporaries into lvalues (i.e. variables).

Could it be that the unfinished analyze::lvalue_temps is supposed to do just that?

@nagisa nagisa added the A-MIR Area: Mid-level IR (MIR) - https://blog.rust-lang.org/2016/04/19/MIR.html label Jan 18, 2016
@nikomatsakis
Copy link
Contributor

I am confused. We do not require temps to be SSA, either in Mir or trans... But in trans we optimize in those cases where we can (I.e., where temps are SSA). Right?

@nagisa
Copy link
Member Author

nagisa commented Jan 18, 2016

@nikomatsakis this comment suggests they are SSA with borrowing and mutation possible.

Might be me misinterpreting what SSA means, but what I mean is that these are not supposed to be assigned more than once, for some definitions of once.

@nikomatsakis
Copy link
Contributor

@nagisa I see. Yes, ok, I agree that comment was in error. I think though that you probably ought to update it in #31077 :)

I would still expect temps to be "DSA" -- which afaik I just made up -- that is, no two writes can reach one another without exiting the "live scope" of the temporary, but I don't know that it's an important property.

FWIW I think SSA usually implies that values are "pure values", i.e., you can't take their address and mutate them. But I know what you mean in any case.

@nikomatsakis
Copy link
Contributor

er, ought to have updated it

@nagisa
Copy link
Member Author

nagisa commented Jan 22, 2016

I’ll keep updating the comment in mind in whatever PR that comes next.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-MIR Area: Mid-level IR (MIR) - https://blog.rust-lang.org/2016/04/19/MIR.html
Projects
None yet
Development

No branches or pull requests

2 participants