Brillvm can't handle multiple Phi nodes

sampsyo / bril

an educational compiler intermediate representation

https://capra.cs.cornell.edu/bril/

MIT License

557 stars 231 forks source link

Brillvm can't handle multiple Phi nodes #326

Closed oflatt closed 4 months ago

oflatt commented 4 months ago

Currently, the rust version of brillvm can't handle multiple phi nodes. This is because LLVM requires all phi nodes to be at the top of the block, and it adds stuff inbetween them.

I've fixed the bug in our fork, but unfortunately we are out of sync with main bril now. Also, my fix is a big ugly. https://github.com/uwplse/bril/pull/4

Pat-Lafon commented 4 months ago

Ah, fair enough. Seems like the stores cause an issue which makes sense. It's weird to use both phi nodes and set up the code to expect a mem2reg pass. You didn't provided any sample test cases so I only tried against the most trivial version but I think my solution is simple enough for you to port if it handles your use case.