Skip to content

Conversation

@vinx13
Copy link
Member

@vinx13 vinx13 commented Sep 26, 2023

This reverts commit 34637d7.

It is better not visiting fields of buffer when visiting MatchBufferRegion since it's more common that the buffer is not mutated in StmtMutator (it can be left to the derived passes to handle case by case). It is also consistent with visitor/mutator for BufferLoad and BufferStore that do not visit buffer fields.
This fixes some broken tests in software pipelining where strides of some buffers in MatchBufferRegion are unintentionally mutated in-place.

cc @yzh119 @tqchen @junrushao

Copy link
Member

@yzh119 yzh119 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I discussed with @vinx13 and I think it's reasonable not to visit buffers in the base mutator but explicitly do that in derived classes if necessary.

@tqchen tqchen merged commit dfd525b into apache:main Sep 26, 2023
@Lunderberg
Copy link
Contributor

As a potential third option, I think that we could have the buffers be visited at their point of definition. That is, any DeclBuffer statement would have the buffer internals visited once, and the mutated buffer would then be substituted in at all points of use. This would require the DeclBuffer to be present for all buffer objects, but that's part of the long-term plan anyways.

vinx13 added a commit to vinx13/tvm that referenced this pull request Oct 17, 2023
…in block visitor functions (apache#15153) (apache#15816)

* Revert "[TensorIR][Visitor] Visit buffer members in `match_buffer`'s in block visitor functions (apache#15153)"
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants