Skip values with existing conversions in getConvertBackwardSlice #8291
Conversation
`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.
neildhar
force-pushed
the
simplify-backward-remat
branch
from
d9ed637 to
8acac09
Compare
|
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 |
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 #8292 stacks both changes, and includes a TTGIR lit test that crashes before the change, but works after it. |
3 participants
getConvertBackwardSlicecurrently 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
I am not making a trivial change, such as fixing a typo in a comment.
I have written a PR description following these
rules.
I have run
pre-commit run --from-ref origin/main --to-ref HEAD.Select one of the following.
/testforlittests/unittestfor C++ tests/python/testfor end-to-end testsSelect one of the following.
littests.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.)