Alias in-place buffers with matching fold factors and strides #604
Conversation
builder/optimizations.cc
Outdated
| @@ -796,8 +797,16 @@ class in_place_aliaser : public stmt_mutator { | |||
| } | |||
| } | |||
|
|
|||
| bool fold_factors_strides_same(const std::vector<dim_expr>& dims_a, const std::vector<dim_expr>& dims_b) { | |||
| for (std::size_t ix = 0; ix < std::min(dims_a.size(), dims_b.size()); ++ix) { | |||
There was a problem hiding this comment.
I think this check needs to guarantee that the allocation being replaced isn't losing a stride/fold_factor constraint, which it might be if it has more dimensions than the alias target.
Basically, I think this check needs to be aware of which dims are the allocation and which are the alias target, and it needs to verify that all of the allocation strides/fold factors match. I also think maybe if the rank doesn't match, it shouldn't allow any stride/fold factor on the allocation (like HEAD).
There was a problem hiding this comment.
Updated the comparison function, PTAL.
There was a problem hiding this comment.
This looks good, but I think this should have a test in cannot_alias.cc, one of the https://github.com/dsharlet/slinky/blob/main/builder/test/cannot_alias.cc#L15 tests that runs twice: once with a constraint that is unproven that prevents aliasing, and once with a constraint that does prove true and allow aliasing.
|
Fixed the comments and added a test. |
|
Review ping. |
builder/test/cannot_alias.cc
Outdated
|
|
||
| var x(ctx, "x"); | ||
|
|
||
| // Here we explicitly use std::functions (in the form of a |
There was a problem hiding this comment.
Pass the functions directly? This looks like copy-paste from a different test
No description provided.