bagel
bagel copied to clipboard
First pass at lowering stuff to an untyped CFG. Very raw.
So, some notes on my own PR:
I'm not happy about an "assign" opcode, that seems pretty wrong. The genesis of that is that local variables are still represented as such here. What's the right way to fix that? Perhaps we just need to lower locals to their SSA reprsentation, then everything can just be an InstructionResult.
I'm also not happy about the tests. I almost wonder if it doesn't make more sense to have some sort of textual representation and use that for testing.
My first reaction would be that you should lower the AST into a CFG that has "assign" opcodes and local variables, and then lower that CFG into a form that doesn't. It can even be the same type, you just assert in between passes that you've successfully lowered all the "assign" node.
Yeah, a textual format and a directory full of "foo.bagel / foo.bagelir" test cases is the best suggestion I have for testing.
I was definitely planining to do two IRs already, one untyped, then one typed. Doing one with locals and one with SSA makes sense I think.
On Wed Nov 12 2014 at 2:22:05 PM Nelson Elhage [email protected] wrote:
My first reaction would be that you should lower the AST into a CFG that has "assign" opcodes and local variables, and then lower that CFG into a form that doesn't. It can even be the same type, you just assert in between passes that you've successfully lowered all the "assign" node.
Yeah, a textual format and a directory full of "foo.bagel / foo.bagelir" test cases is the best suggestion I have for testing.
— Reply to this email directly or view it on GitHub https://github.com/alex/bagel/pull/69#issuecomment-62756024.
Yeah, my belief is that you'll want (at least) two IRs, but even within a single IR type, you may want different "flavors" of it, that have different invariants -- e.g. "in / not in SSA form", "have removed all nodes of type X", etc. You could make all of them different types, but they'll be similar enough that it's not worth it.
Ok, I think I'm reasonable happy with teh scope of this patch at this point. Right now it demonstrates:
- locals
- conditional branches
- some arithmetic
Is there anything I'm missing that would substantially affect how this patch would work?