thorin2 icon indicating copy to clipboard operation
thorin2 copied to clipboard

Bugfix/clos conv

Open leissa opened this issue 2 years ago • 2 comments

Just a draft. Trying to fix #117 and #126 and along the way port the phases from thorin::Phase to thorin::RWPhase to reduce some boilerplate.

leissa avatar Nov 06 '22 15:11 leissa

I do not know the inner workings of the closure conversion pass.

But in general, it might be useful to solve this issue by adding a separate pass in mem or clos that propagates mem and adds it everywhere.

Together with a (fixed) version of reshape/scalerize that makes everything flat and a (maybe separate from the previous) pass that reorders arguments, this should be enough to fix this issue.

The added benefit would be that operations can request pseudo-memory nodes out of thin air. This allows optimizing more freely. The forced (partial) ordering could still be enforced with the pseudo-memory elements. The memory-propagation pass (or an additional pass) would replace them.

Nevertheless, it might still be helpful to make closure conversion more resistant depending on its current state in a next step.

NeuralCoder3 avatar Nov 06 '22 15:11 NeuralCoder3

Together with a (fixed) version of reshape/scalerize that makes everything flat and a (maybe separate from the previous) pass that reorders arguments, this should be enough to fix this issue.

As an update / for reference: The reshape improvements and add-mem tests happen in the ho_codegen branch using optimizations from feature/add-mem.

The pseudo memory could probably just be ⊥:%mem.M which is replaced by the memory sweep in the add-mem phase.

NeuralCoder3 avatar Nov 18 '22 15:11 NeuralCoder3