-
-
Notifications
You must be signed in to change notification settings - Fork 21
Introduce a new compiler pass: MIR #182
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
Conversation
|
Alright, this is ready for review. Not saying it's great, but I need a second pair of eyes on it now. All the tests pass at least. |
|
I'll list out some notable changes I made along the way:
|
bal-e
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Amazingly well done, @tertsdiepraam! This is a major change and it's going to make it way easier to develop Roto further. I've left some minor notes, but they can all be addressed later.
| /// Integer addition | ||
| Add { | ||
| to: Var, | ||
| left: Operand, | ||
| right: Operand, | ||
| }, | ||
|
|
||
| /// Integer subtraction | ||
| Sub { | ||
| to: Var, | ||
| left: Operand, | ||
| right: Operand, | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see FAdd or FSub... do these instructions cover floating-points too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently the codegen step makes that distinction, div is different because it needs info on signedness for integers. I should refactor this to a binop instruction or something. But that's for later.
src/lir/print.rs
Outdated
| label_store: printer.label_store, | ||
| }; | ||
|
|
||
| s.push_str(&format!( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe use write!(&mut s, ...)?
| @@ -0,0 +1,99 @@ | |||
| //! Dead code elimination on the MIR | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This only eliminates dead branches, not dead instructions within used blocks, right? That'll be a lot harder to do, but also a lot more interesting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that's true
src/mir/mod.rs
Outdated
| @@ -0,0 +1,156 @@ | |||
| //! Mid-level intermediate representation (MIR) | |||
| //! | |||
| //! This is the first intermediate representation. It is baed on basic blocks | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo: 'baed' -> 'based'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
based comment
No description provided.