Skip values with existing conversions in getConvertBackwardSlice
getConvertBackwardSlice currently includes any existing rematerialisation it finds in the returned slice. However, this shouldn't be necessary because that value does not need to be rematerialised or included in the cost calculation.
Instead, once we find a valid rematerialisation, we can stop the traversal right away.
New contributor declaration
-
[x] I am not making a trivial change, such as fixing a typo in a comment.
-
[x] I have written a PR description following these rules.
-
[x] I have run
pre-commit run --from-ref origin/main --to-ref HEAD. -
Select one of the following.
- [ ] I have added tests.
/testforlittests/unittestfor C++ tests/python/testfor end-to-end tests
- [x] This PR does not need a test because it is a simplification and existing tests pass.
- [ ] I have added tests.
-
Select one of the following.
- [x] I have not added any
littests. - [ ] The
littests I have added follow these best practices, including the "tests should be minimal" section. (Usually running Python code and using the instructions it generates is not minimal.)
- [x] I have not added any
Can you add a lit test that shows an improvement with this PR?
@peterbell10 I'm not sure it's possible to observe an improvement from this since the existing conversion is almost always itself a convert_layout. This PR is intended to simplify and document the interface to set up the bug fix in https://github.com/triton-lang/triton/pull/8292.
@peterbell10 I'm not sure it's possible to observe an improvement from this since the existing conversion is almost always itself a
convert_layout. This PR is intended to simplify and document the interface to set up the bug fix in #8292.
are you saying it is an NFC change? If so can you stack the future changes somewhere so that we can review it together?
@ThomasRaoux Yes, I believe the way the pass is currently set up, this doesn't have any effect. My second PR https://github.com/triton-lang/triton/pull/8292 stacks both changes, and includes a TTGIR lit test that crashes before the change, but works after it.
This was combined with https://github.com/triton-lang/triton/pull/8292 and merged in e3cf2ad0b57604f31e18455794c41772ce31f21d.
The PR stacked on top of this (https://github.com/triton-lang/triton/pull/8292) was reverted due to regressions. I'm not sure whether the changes in this PR were responsible. If not, @peterbell10, would you consider merging this PR independently?