Initial wiring for memory into RVSDGs

egraphs-good / eggcc

MIT License

51 stars 11 forks source link

Initial wiring for memory into RVSDGs #305

Closed ezrosent closed 9 months ago

ezrosent commented 9 months ago

The structure for these memory operations is extremely similar to prints, with all operations "listening" to the same state edge introduced for prints.

This PR does not touch any of the conversions to and from egglog in either encoding.

ezrosent commented 9 months ago

Sorry, I'm not quite sure what you're asking here. How would we compute the value of the "supports_memory" filter? AIUI the desired flow would be:

Add new bril / insta tests for memory.
Run all the memory-involving tests on modes where supports_memory=true (just RvsdgConversion for now?)

Would it be easier to add those tests once memory operations are wired into bril, etc? (I'm also not against GCing more of the old run modes too). Then we could just run them uniformly everywhere, with unit tests ensuring we have test coverage until then.

oflatt commented 9 months ago

Yeah, I'm proposing adding in some tests for memory from Bril's test suite and just run RvsdgConversion on them. In the past, running on these files in addition to unit tests has uncovered lots of bugs.

I agree it would be easier if all run modes supported memory, but it'll be a while until round-trip with memory works. Yeah, I'm hoping (real soon!) we can delete all code not related to our favorite schema.

ezrosent commented 9 months ago

Added the memory tests from bril. This did uncover a bug in SVG generation that I patched!